Introduce MissingRefasterAnnotation checker
#2232
Conversation
| .distinct() | ||
| .count(); | ||
|
|
||
| return methodTypes < 2 ? Description.NO_MATCH : buildDescription(tree).build(); |
There was a problem hiding this comment.
Why only match if there are at least two methods with refaster annotations? I know we'd usually expect to see at least two (e.g. both a @BeforeTemplate and @AfterTemplate) but would only matching on one result in false positives?
Maybe there are cases where there's an abstract class that's used to share a placeholder or something?
There was a problem hiding this comment.
We want to partition the methods into two groups: those with a Refaster annotation and those without.
The checker should only flag classes that have methods of both types.
So far, we have not seen a scenario in which a Refaster template class contains an unannotated method.
If those do exist, can you share a concrete example, perhaps from Google's vast internal collection of Refaster templates?
|
|
||
| @Override | ||
| public Description matchClass(ClassTree tree, VisitorState state) { | ||
| long methodTypes = |
There was a problem hiding this comment.
I'd consider implementing this is a MethodTreeMatcher instead of a ClassTreeMatcher that scans its members for methods, but that depends on whether the check for < 2 methods below is necessary.
There was a problem hiding this comment.
Perhaps a better name would be methodTypeCount.
Since we want to know about all the methods in the class, we use a ClassTreeMatcher.
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
0976cd2
to
965fb2f
Compare
|
Rebased and added a commit. What do you think of the check? |
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
965fb2f
to
3e0411f
Compare
|
Rebased and added a small commit. Still think this check can be useful :). |
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
3e0411f
to
d6a7be7
Compare
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
d6a7be7
to
981dcbe
Compare
|
Rebased. |
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
981dcbe
to
ce982d8
Compare
|
Rebased. |
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
ce982d8
to
4eb1c41
Compare
rickie
force-pushed
the
rossendrijver/bugpattern_refaster_annotations
branch
from
4eb1c41
to
211966e
Compare
This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [com.google.errorprone:error_prone_core](https://errorprone.info) ([source](https://github.com/google/error-prone)) | | minor | `2.19.1` -> `2.20.0` | | [com.google.errorprone:error_prone_annotations](https://errorprone.info) ([source](https://github.com/google/error-prone)) | compile | minor | `2.19.1` -> `2.20.0` | --- ### Release Notes <details> <summary>google/error-prone</summary> ### [`v2.20.0`](https://github.com/google/error-prone/releases/tag/v2.20.0): Error Prone 2.20.0 [Compare Source](google/error-prone@v2.19.1...v2.20.0) Changes: - This release is compatible with early-access builds of JDK 21. New Checkers: - [`InlineTrivialConstant`](https://errorprone.info/bugpattern/InlineTrivialConstant) - [`UnnecessaryStringBuilder`](https://errorprone.info/bugpattern/UnnecessaryStringBuilder) - [`BanClassLoader`](https://errorprone.info/bugpattern/BanClassLoader) - [`DereferenceWithNullBranch`](https://errorprone.info/bugpattern/DereferenceWithNullBranch) - [`DoNotUseRuleChain`](https://errorprone.info/bugpattern/DoNotUseRuleChain) - [`LockOnNonEnclosingClassLiteral`](https://errorprone.info/bugpattern/LockOnNonEnclosingClassLiteral) - [`MissingRefasterAnnotation`](https://errorprone.info/bugpattern/MissingRefasterAnnotation) - [`NamedLikeContextualKeyword`](https://errorprone.info/bugpattern/NamedLikeContextualKeyword) - [`NonApiType`](https://errorprone.info/bugpattern/NonApiType) Fixes issues: [#​2232](google/error-prone#2232), [#​2243](google/error-prone#2243), [#​2997](google/error-prone#2997), [#​3301](google/error-prone#3301), [#​3843](google/error-prone#3843), [#​3903](google/error-prone#3903), [#​3918](google/error-prone#3918), [#​3923](google/error-prone#3923), [#​3931](google/error-prone#3931), [#​3945](google/error-prone#3945), [#​3946](google/error-prone#3946) **Full Changelog**: google/error-prone@v2.19.1...v2.20.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
See the
MissingRefasterAnnotation.mdfor more details.Feedback is welcome!