ASTHelpersSuggestions replacement is missing the class name `ASTHelpers`

[mernst]

This Error Prone suggestion is incorrect:

TreeFinder.java:278: warning: [ASTHelpersSuggestions] Prefer ASTHelpers instead of calling this API directly
              if (jfa.sym.isStatic()) {
                                  ^
    (see https://errorprone.info/bugpattern/ASTHelpersSuggestions)
  Did you mean 'if (isStatic(jfa.sym)) {'?

The replacement should be ASTHelpers.isStatic(...) rather than isStatic(...).

Also, it would be helpful for the documentation to indicate where to find the ASTHelpers class (i.e., how to get it on the classpath so that the replacement compiles).

[rickie]

In the ASTHelperSuggestions the addStaticImport method is used here. This means that the suggested replacement is actually correct. This is also shown and verified in the test class ASTHelpersSuggestionsTest.

The ASTHelpers class is located in the check_api module, see here.

I agree that it might be helpful to add some documentation about where to find the ASTHelpers class.

[mernst]

In the ASTHelperSuggestions the addStaticImport method is used here. This means that the suggested replacement is actually correct.

The suggested replacement (in the Error Prone output that I quoted) is not correct. However, it's good to hear that the replacement performed by a tool (which differs from the suggested replacement in adding a static import) is correct. Thanks for pointing that out.

I agree that it might be helpful to add some documentation about where to find the ASTHelpers class.

And about the need to add the static import.

[graememorgan]

The canonical fix suggested by a check is the one that rickie@ is referring to, generated via a Fix interface. We surface that internally in tooling at review time at Google, and in the OSS version you can apply generated patches with the CLI. I'm much less familiar with that, though!

The message shown in "Did you mean ...?" is a suggestive portion of the Fix. It's not generally complete, in that it doesn't include imports, or more than the first line (I think).

[rickie]

It's not generally complete, in that it doesn't include imports, or more than the first line (I think).

You are right @graememorgan 😄.