Issue 543 by phisad · Pull Request #2252 · spotbugs/spotbugs

phisad · 2022-11-09T15:59:53Z

Hello all,

I added a simple AnnotationMatcher that matches classes annotated with that annotation name.
For example, use in an excludeFilter.xml as follows

<Match>
<Annotation name="org.immutables.value.Generated" />
</Match>

to ignore classes generated by the Immutable framework.

Request For Comments
The main issue was to add the java class annotation names of the analysed classes to the PackageMemberAnnotation for later use during exclude file filtering. For this I simply changed the BugInstance.add() method as this seemed to me a central point to get this information and avoiding to re-write all the detectors. For better performance I access the IAnalysisCache.

Please suggest a better place for this, if necessary, because I do not know the codebase too well.

The PR includes:

a new AnnotationMatcher for excludeFilter.xml files; added an AnnotationMatcherTest
add AnnotationType to findbugsfilter.xsd
add a javaAnnotationNames attribute to PackageMemberAnnotation
added addJavaAnnotationNames() method to BugInstance as a central point to get the java annotations that a class is annotated with
a comma-separated "classjas" attribute is now written to XML when writeXml() is called on ClassAnnotation, MethodAnnotation or FieldAnnotation; a test case is added to BugInstanceTest
adjust SAXBugCollectionHandler to handle the new "Annotation" matcher and "classjas" attribute; a test case added to SAXBugCollectionHandlerTest

Make sure these boxes are checked before submitting your PR -- thank you!

Added an entry into CHANGELOG.md if you have changed SpotBugs code

ThrawnCA · 2022-11-10T23:20:47Z

Build is failing due to a formatting error.

phisad · 2022-11-16T16:31:19Z

Build is failing due to a formatting error.

I ran spotlessApply on spotbugs-spotbugs and spotbugs-test and hope it is fixed now.

phisad · 2022-11-18T11:43:33Z

Fixed the test and run spotlessApply and test

phisad · 2022-11-25T12:43:08Z

btw. any comments on the changes to BugInstance.add() ?

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/filter/AnnotationMatcherTest.java

spotbugs/src/main/java/edu/umd/cs/findbugs/BugInstance.java

phisad · 2022-11-30T15:06:25Z

Thanks for the valuable comments. I applied the requested changes.

hazendaz · 2023-01-05T01:41:23Z

spotbugs/src/main/java/edu/umd/cs/findbugs/ClassAnnotation.java

        }

+        if (!getJavaAnnotationNames().isEmpty()) {
+            attributeList.addAttribute("classjas", // similar to classname, short for classJavaAnnotations


@phisad is 'classjas' just made up naming? Seems odd even from change log. Why not just 'classAnnotations'? We already know its java, that is redundant. At least then it follows the idea you have there and is more clear on name without new jargon. Then you probably don't need to describe why its named that way...

Thanks for the comment! I was also thinking about "classAnnotations" but felt that it might be confused with the ClassAnnotation from spotbugs. But I think you are right, that a user of the XML file will not know that the term "classAnnotation" is also otherwise used. If you say that this is clearer, then I will certainly adjust it to "classannotation" (to comply with classname which is also not camelcase).

Good point, thanks for calling that out, that certainly would cause unnecessary confusion. how about 'classAnnotationNames' which is more in line with what its loading there?

OK, I changed the attribute name in the XMLs from classjas to classAnnotationNames

ThrawnCA · 2023-01-15T23:10:43Z

spotbugs/etc/findbugsfilter.xsd

+        </annotation>
+        <attribute name="name" type="string">
+            <annotation>
+                <documentation>The exact or regex match pattern for a java class annotation name. If the name starts with the ~ character the rest of attribute content is interpreted as a Java regular expression.</documentation>


"the rest of attribute content" -> "the rest of the attribute content

Adjusted. Note that this is a copy paste error (that still exists in the other documentation tags as well).

ThrawnCA · 2023-01-15T23:11:19Z

spotbugs/src/main/java/edu/umd/cs/findbugs/BugInstance.java

+                // map annotation entry type to dotted class name, for example
+                // Lorg/immutables/value/Generated; -> org.immutables.value.Generated
+                String annotationType = ae.getAnnotationType().substring(1); // remove starting L
+                annotationType = annotationType.replace("/", ".");


These two lines could be combined.

Inlined the replace operations.

hazendaz · 2023-01-30T17:20:53Z

CHANGELOG.md


-#### Security
+### Added
+* New simple name-based AnnotationMatcher for exclude files (now bug annotations store the class java annotations in an attribute called `classjas`). For example, use like <Match><Annotation name="org.immutables.value.Generated" /></Match> in an excludeFilter.xml to ignore classes generated by the Immutable framework. This ignores all class, method or field bugs in classes with that annotation.


@phisad Please fix the classjas part here too, please double check to make sure all is where needed then I think we will be good.

ThrawnCA · 2023-01-30T23:53:53Z

Changelog needs to be deconflicted and merged.

phisad · 2023-02-10T11:14:20Z

Rebased on upstream/master. Adjusted CHANGELOG.

hazendaz · 2023-02-22T15:32:40Z

@phisad Fix fix formatting issues GH-actions raised.

The following files had format violations:
src/main/java/edu/umd/cs/findbugs/BugInstance.java
@@ -2113,7 +2113,7 @@
············List·javaAnnotationNames·=·Arrays.asList(annotationEntries).stream().map((AnnotationEntry·ae)·->·{
················//·map·annotation·entry·type·to·dotted·class·name,·for·example
················//·Lorg/immutables/value/Generated;·->·org.immutables.value.Generated
-················String·annotationType·=·ae.getAnnotationType().substring(1).replace("/",·".").replace(";",·"");·
+················String·annotationType·=·ae.getAnnotationType().substring(1).replace("/",·".").replace(";",·"");
················return·annotationType;
············}).collect(Collectors.toList());
············pma.setJavaAnnotationNames(javaAnnotationNames);
Run './gradlew :spotbugs:spotlessApply' to fix these violations.

phisad · 2023-02-27T10:33:33Z

Did run './gradlew :spotbugs:spotlessApply' to fix the violations.

phisad · 2023-03-29T10:19:25Z

@hazendaz Does it look fine now?

hazendaz · 2023-03-29T23:27:18Z

spotbugs/etc/findbugsfilter.xsd

+
+    <element name="Annotation" type="fb:AnnotationType">
+        <annotation>
+            <documentation>This element matches warnings associated with a particular class that is annoted with a 


@phisad See a spelling mistake here, should 'annoted' be 'annotated'?

hazendaz · 2023-03-29T23:29:26Z

@phisad Overall good, just one spelling mistake I found, can you do a double check across this?

@ThrawnCA Look good to you now as well?

cpfeiffer · 2023-03-31T09:30:42Z

Supserseded by #2395

pzygielo mentioned this pull request Nov 16, 2022

Support for filtering by annotation #543

Closed

ThrawnCA requested changes Nov 28, 2022

View reviewed changes

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/filter/AnnotationMatcherTest.java Outdated Show resolved Hide resolved

spotbugs/src/main/java/edu/umd/cs/findbugs/BugInstance.java Outdated Show resolved Hide resolved

ThrawnCA approved these changes Dec 1, 2022

View reviewed changes

hazendaz reviewed Jan 5, 2023

View reviewed changes

ThrawnCA requested changes Jan 15, 2023

View reviewed changes

hazendaz reviewed Jan 30, 2023

View reviewed changes

ThrawnCA approved these changes Jan 30, 2023

View reviewed changes

Philipp Sadler added 16 commits February 10, 2023 12:02

Issue-543 Store Java class annotation names to PackageMemberAnnotations

4cc9ff7

Issue-543 Store Java class annotation names to PackageMemberAnnotations

6e2ba31

Issue-543 Add AnnotationMatcher for filtering (with test cases)

29e30b6

Issue-543 Extend SAX handler for AnnotationMatcher and classjas attr

e274823

Issue-543 Fix whitespaces

f34b59f

Issue-543 Add CHANGELOG entry

e091a72

Issue-543 Make compatible with Java 1.8

cce5bef

Issue-543 Extract code to addJavaAnnotationNames method

60fcaf6

Issue-543 Update filter file xml schema

dec1b32

Issue-543 Run spotlessApply

1881b31

Issue-543 Fix AnnotationMatcherTest

6bab352

Issue-543 Fix file header comments

e756ba2

Issue-543 Use MethodHandles for logger

29aedae

Issue 543 Fix documentation

cd5b37a

Issue 543 Combine expressions when getting annotation type

5386915

Issue 543 Rename attribute classjas to classAnnotationNames

4d33197

phisad force-pushed the issue-543 branch from 42ff4ce to 4d33197 Compare February 10, 2023 11:11

Issue-543 Fix whitespaces

f009905

hazendaz reviewed Mar 29, 2023

View reviewed changes

hazendaz self-assigned this Mar 29, 2023

cpfeiffer mentioned this pull request Mar 31, 2023

Issue 543 #2395

Merged

hazendaz closed this in #2395 Apr 2, 2023

Conversation

phisad commented Nov 9, 2022

Uh oh!

ThrawnCA commented Nov 10, 2022

Uh oh!

phisad commented Nov 16, 2022

Uh oh!

phisad commented Nov 18, 2022

Uh oh!

phisad commented Nov 25, 2022

Uh oh!

Uh oh!

Uh oh!

phisad commented Nov 30, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phisad Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phisad Jan 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThrawnCA commented Jan 30, 2023

Uh oh!

phisad commented Feb 10, 2023

Uh oh!

hazendaz commented Feb 22, 2023

Uh oh!

phisad commented Feb 27, 2023

Uh oh!

phisad commented Mar 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hazendaz commented Mar 29, 2023

Uh oh!

cpfeiffer commented Mar 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

phisad Jan 13, 2023 •

edited

Loading

phisad Jan 30, 2023 •

edited

Loading