Enable SuppressWarnings annotation #74
Conversation
| makeFile(new File(outputDir, | ||
| ModernizerAnnotationUtils.IGNORE_CLASSES_FILE_NAME), | ||
| annotatedElements.getAnnotatedClasses()); | ||
| makeFile(new File(outputDir, |
There was a problem hiding this comment.
Could you add a comment about what these files do?
| for (String element : annotatedElements) { | ||
| writer.write(element + "\n"); | ||
| } | ||
| writer.close(); |
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
| } finally { | ||
| if (writer != null) { |
There was a problem hiding this comment.
Utils is in the maven-plugin module, whereas this code is in the annotation-processor module. Do you want me to move it?
| for (TypeElement annotation : annotations) { | ||
| AnnotatedElements annotatedElements = | ||
| getAnnotatedElements(roundEnv, annotation); | ||
| if (!(annotatedElements.getAnnotatedClasses().isEmpty() && |
There was a problem hiding this comment.
While this means the same thing, it might read better as !isEmpty || !isEmpty.
| try { | ||
| writer = new BufferedWriter(new FileWriter(file)); | ||
| for (String element : annotatedElements) { | ||
| writer.write(element + "\n"); |
There was a problem hiding this comment.
Avoid StringBuilder via:
writer.write(element);
writer.write("\n");
modernizer-maven-plugin/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.google.guava</groupId> | ||
| <artifactId>guava-base</artifactId> | ||
| <version>r03</version> |
There was a problem hiding this comment.
What does this dependency do given the above?
|
@gaul I believe I've addressed all your comments about the code and added a question where the best approach was not clear. Let me know what you think. I apologize for the lengthy delay in responding: Nimisha was a co-op whose term ended and I was on leave for the past couple of months. |
|
This is a decent approach, though I wonder if it wouldn't be simpler to introduce another annotation with Please also note that the correct way to use an annotation processor is the recently added |
Could we explore this simpler approach? Sorry that I have been tardy with reviewing but don't understand much about annotation processing. |
|
Can someone please close this PR? I'm opening a new one with |
|
New PR: #86 |
|
Pursuing a different approach in #86. |
Addresses #3
This PR allows users to suppress modernizer errors by adding
@SuppressWarnings("modernizer")at class/method level.@gaul
cc @jhaber @stevegutz
How can a user suppress modernizer warnings?
modernizer-annotation-processorshould be added as a provided-scoped dependency in the project.Any class or method can be annotated with
@SuppressWarnings("modernizer")to suppress all modernizer errorsin that block of code.
Implementation strategy
modernizer-maven-pluginhas the main plugin code.modernizer-maven-policyhas checkstyle rules and is added as a dependency tomaven-checkstyle-pluginin the plugin's pom.
modernizer-annotation-processorhas annotation processing code and is added as a dependency in plugin.methods/classes annotated as
@SuppressWarnings("modernizer"). These annotations can be added to methods or classes.annotated class and its subclasses.
For example:
org/gaul/package/ClassOne\$ClassTwo(\$.+)?qualified class name, the method name, the return type, and the list of parameters. These are normalized to be compared to the format
expected by ASM.
For example:
org/gaul/package/ClassOne methodOne int[] java.util.Listignore-annotated-classes.txtandignore-annotated-methods.txt. These files are created in the/target/modernizer/testand/target/modernizer/maindirectories of the processing environment. Adding the files to these directories makes them accessible to the plugin while excluding them
from the jar of the project being built.
Testing strategy
ModernizerSuppressionsEndToEndTestClasshas helper classes/methods annotated with@SuppressWarnings("modernizer").The processor scans this class before compiling and creates the ignore files.
End-to-end flow tests are added in
ModernizerSuppressionsEndToEndTestwhich read the ignore files and check ifviolations in the code are as expected.
We have benchmarked the plugin after this implementation, and haven't seen any performance issues.
Further improvements
ignore-methodsfile could be improved by making it more structured by using XML, JSON, or YAML.@SuppressWarnings("modernizer")blocks all modernizer errors in that code block. Suppressing specific checks/errors canbe done by adding something like
<id>ViolationIdentifier</id>to the violations file structure, which uniquelyidentifies each violation. We can then use
@SuppressWarnings("modernizer:ViolationIdentifier")to suppress errorsrelated to those particular checks in the code block.