将CheckReturnValue应用于整个项目。

huangapple go评论81阅读模式
英文:

apply CheckReturnValue to entire project

问题

我在一个大型的遗留Java 8(Android)应用程序上工作。我们最近发现了一个由于方法忽略结果而引起的错误。具体地说,一个send()方法的调用者在发送失败时没有采取正确的操作。这个问题已经修复了,但现在我想添加一些静态分析来帮助找出我们代码中是否存在同类性质的其他现有错误。此外,还要防止将来添加同类性质的新错误。

我们已经在使用Find Bugs、PMD、Checkstyle、Lint和SonarQube。因此,我认为其中一个工具可能已经具备我需要的检查,只需要启用它。但经过几个小时的搜索和测试后,我认为这不是情况。

以下是我正在测试的代码:

public class Application {
    public status void main(String[] args) {
        foo(); // 我希望这个被捕捉到
        Bar aBar = new Bar();
        aBar.baz(); // 我希望这个被捕捉到
    }
    
    static boolean foo() {
        return System.currentTimeMillis() % 2 == 0;
    }
}

public class Bar {
    boolean baz() {
        return System.currentTimeMillis() % 2 == 0;
    }
}

我想在调用者端捕捉这个问题,因为有些调用者可能会使用返回值,而其他调用者则不会(上面描述的send()方法就是这种情况)。

我找到了以下现有的静态分析规则,但它们似乎只适用于非常特定的情况,以避免误报,并不适用于我的示例:

到目前为止,最好的选择似乎是#3,但这需要我在我的大型项目中为每个方法或类都添加注释。Java 9+似乎允许在包级别添加注释,但这对我来说不是选择。即使可以,这个项目有很多包。我真的想找到一种方法,可以通过一个或几个地方配置此规则,而不是需要修改每个文件。

最后,我发现IntelliJ的这个Stack Overflow答案显示了它具有"报告所有被忽略的非库调用"的检查选项。在IDE中执行此操作似乎有效。但我希望这会导致CI失败。我发现有一种方法可以通过命令行使用IntelliJ工具来触发此操作,但这仍然会输出一个XML/JSON文件,我需要编写自定义代码来解析该输出。我还需要在CI机器上安装IDE工具,这似乎有些过于复杂。

有没有人知道实现我想要的更好方法?我不可能是第一个只关心假阴性而不关心假阳性的人。我觉得应该能够使当前未使用的任何返回值要么被记录,要么通过注释或将其分配给一个变量的约定明确声明返回值被有意忽略,就像Error Prone中所做的那样。

英文:

I work on a large legacy Java 8 (Android) application. We recently found a bug that was caused by an ignored result of method. Specifically a caller of a send() method didn't take the right actions when it the sending failed. It's been fixed but now I want to add some static analysis to help find if other existing bugs of the same nature exist in our code. And additionally, to prevent new bugs of the same nature from being added in the future.

We already use Find Bugs, PMD, Checkstyle, Lint, and SonarQube. So I figured that one of these probably already has the check I'm looking for, but it just needs to be enabled. But after a few hours of searching and testing, I don't think that's the case.

For reference, this is the code I was testing with:

public class Application {
    public status void main(String[] args) {
        foo(); // I want this to be caught
        Bar aBar = new Bar();
        aBar.baz(); // I want this to be caught
    }
    
    static boolean foo() {
        return System.currentTimeMillis() % 2 == 0;
    }
}

public class Bar {
    boolean baz() {
        return System.currentTimeMillis() % 2 == 0;
    }
}

I want to catch this on the caller side since some callers may use the value while others do not. (The send() method described above was this case)

I found the following existing static analysis rules but they only seem to apply to very specific circumstances to avoid false positives and not work on my example:

So far the best option seems to be #3 but it requires me to annotate EVERY method or class in my HUGE project. Java 9+ seems to allow annotating at the package-level but that's not an option for me. Even if it was, the project has A LOT of packages. I really would like a way to configure this to be applied to my whole project via one/few locations instead needing to modify every file.

Lastly I came across this Stack Overflow answer that showed me that IntelliJ has this check with a "Report all ignored non-library calls" check. Doing this seems to work as far as highlighting in the IDE. But I want this to cause CI fail. I found there's a way to trigger this via command line using intelliJ tools but this still outputs an XML/JSON file and I'll need to write custom code to parse that output. I'd also need to install IDE tools onto the CI machine which seems like overkill.

Does anyone know of a better way to achieve what I want? I can't be the first person to only care about false negatives and not care about false positives. I feel like it should be manageable to have any return value that is currently being unused to either be logged or have it explicitly stated that the return value is intentionally ignored it via an annotation or assigning to a variable convention like they do in Error Prone

答案1

得分: 1

像您描述的这种情况往往会导致严重的软件缺陷(在各个方面都是真正的错误);更加令人沮丧和棘手的是,代码会悄无声息地失败,并且允许问题保持隐藏。您希望识别类似的隐藏缺陷(并纠正它们)是可以理解的;然而,(我谦虚地建议)静态代码分析可能不是最佳策略:

  • 根据您在问题中表达的担忧:CheckReturnValue 规则存在很高的风险,会产生大量的 //Ignore 代码注释、规则 violationSuppress 子句和/或 @suppressRule 注解,远远超过规则的积极缺陷检测计数。

  • Java 编程语言进一步增加了规则抑制计数的可能性,考虑到 Java 垃圾回收并评估垃圾回收对软件开发的影响。从理解 Java 垃圾回收基于对象实例引用计数的角度出发,只有引用计数为 0(零)的实例才有资格进行垃圾回收,对于 Java 开发人员来说,避免不必要的引用并自然地忽略不重要的方法调用返回值是完全合理的做法。被忽略的实例将简单地从本地调用堆栈中移除,大多数实例将达到引用计数为 0(零),立即有资格进行垃圾回收。

现在从负面角度转向正面,我提供了一些供您考虑的替代方案,我相信这些方案将提高您的结果以及达到成功结果的可能性:

  • 根据您对情境和结果缺陷/错误的描述,似乎问题的直接根本原因是单元测试失败或集成测试失败。对于可能(并且几乎肯定在某个时候会)失败的发送操作的实现,无论是单元测试还是集成测试都绝对应该考虑到多种可能的失败情况,并验证了失败情况的处理。我显然不知道,但我愿意打赌,如果您专注于创建和运行单元测试和集成测试,系统的质量将在每一步都得到改善,这些改进将清晰可见,您很可能会发现当前引起您关切、忧虑、紧张和担忧的一些或全部隐藏错误的原因。

  • 考虑保持您当前静态代码分析研究的要点,但将方法转向新方向。第一次阅读您的问题时,我突然意识到您想执行的代码检查存在于代码库中的多个无关地点,并且正在迅速变得过于复杂,不同代码段的具体细节不同,每个特殊情况都使整体工作变得不切实际。基本上,您想要实现的东西代表了一个跨越代码库相当大部分的横切目标,而实现细节已经使一个相当简单的好主意变得荒谬复杂。您的问题几乎是最适合采用横切面向的问题的教科书式示例。

如果您有时间和兴趣,请考虑查看 AspectJ 框架,也许编写一些探索性横切面,并让我知道您的想法。我很愿意听听您的想法,如果您愿意随时进行一次极客开发的对话。我真诚地希望这对您有所帮助-

英文:

Scenarios like the one you describe invariably give rise to a substantial software defect (a true bug in every respect); made more frustrating and knotty because the code fails silently, and which allowed the problem to remain hidden. Your desire to identify any similar hidden defects (and correct them) is easy to understand; however, (I humbly suggest) static code analysis may not be the best strategy:

  • Working from the concerns you express in your question: a CheckReturnValue rule runs a high risk of producing a cascade of //Ignore code comments, rule violationSuppress clauses, and/or @suppressRule annotations that far outnumber the rule's positive defect detection count.

  • The Java programming language further increases the likelihood of a high rule suppression count, after taking Java garbage collection into consideration and assessing how garbage collection effects software development. Working from the understanding that Java garbage collection is based on object instance reference counting, that only instances with a reference count of 0 (zero) are eligible for garbage collection, it makes perfect sense for Java developers to avoid unnecessary references, and to naturally adopt the practice of ignoring unimportant method call return values. The ignored instances will simply fall off of the local call stack, most will reach a reference count of 0 (zero), immediately become eligible for and quickly undergo garbage collection.

Shifting now from a negative perspective to positive, I offer alternatives, for your consideration, that (I believe) will improve your results, as well as your probability to reach a successful outcome.

  • Based on your description of the scenario and resulting defect / bug, it feels like the proximate root cause of the problem is a unit testing failure or an integration testing failure. The implementation of a send operation that may (and almost certainly will at some point) fail, both unit testing and integration testing absolutely should have incorporated multiple possible failure scenarios and verified failure scenario handling. I obviously don't know, but I'm willing to bet that if you focus on creating and running unit tests and integration tests, the quality of the system will improve at every step, the improvements will be clearly evident, and you may very well uncover some or all of the hidden bugs that are the cause of your current concern, trepidation, stress, and worry.

  • Consider keeping the gist of your current static code analysis research alive, but shift your approach in a new direction. The first time I read your question, I was struck by the realization that the code checks you would like to perform exist in multiple unrelated locations across the code base and are quickly becoming overly complex, the specific details of the checks are different in many section of code, and each of the special cases make the overall effort unrealistic. Basically, what you would like to implement represents a cross-cutting goal that falls across a sizable section of the code base, and the implementation details have made what is a fairly simple good idea ridiculously complex. Your question is almost a textbook example of a problem that is best implemented taking a cross-cutting aspect-oriented approach.

If you have the time and interest, please take a look at the AspectJ framework, maybe code a few exploratory aspects, and let me know what you think. I'd like to hear your thoughts, if you feel like having a geeky dev conversation at some point. I really hope this is helpful-

答案2

得分: 1

你可以使用intelliJ IDEA的检查功能:Java | 可能的错误 | 忽略方法调用的结果,并启用“报告所有被忽略的非库调用”选项。它会捕捉到你的代码示例中提供的两种情况。

英文:

You may use the intelliJ IDEA's inspection: Java | Probable bugs | Result of method call ignored with "Report all ignored non-library calls" option enabled. It catches both cases provided in your code sample.

huangapple
  • 本文由 发表于 2020年8月7日 04:59:09
  • 转载请务必保留本文链接:https://go.coder-hub.com/63291655.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定