migrate SuppressWarningsHolder to use property macro #13839
Comments
|
@stoyanK7 , can you help us to fix this issue? |
|
Property Setter: No field. XDocs Overrides: checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/internal/XdocsPagesTest.java Lines 1019 to 1021 in aab6311 |
|
@romani |
|
Google doxia and macro to look at its documentation. It is not something we made and I don't know the location for it. The PropertiesMacro is ours, and you can find it in our repository under that name. |
|
I was looking into the PropertiesMacro file. And, can this be a way to proceed? The aim could be to extend the functionality to handle properties that do not have a corresponding Field, the ones who could be accessible via setter methods like aliasList, without changing the existing functionality (backwards compatibility) i.e. having a provision for field or setter-method level retrival. Or, the solution is simple than i think it to be? |
|
@relentless-pursuit , please check comments on similar issue #13867 (comment) There is some limitations and hardcoded mapping parent classes. Solution somewhere there. |
|
This check has no parent classes. Property is in the same class. |
|
In this case we need to extend logic to check for setters if no field is found |
@romani |
|
Yes, but is it is not doxia parse method, it is general reflection method to find field/setter. |
I have studied this function. I think the solution to fix this error is by re-naming the function. Why should we re-name?Its pretty logical that there is no real @romani - let me know if you are ok with this idea? |
|
Renaming a property setter to anything else will break the check as well as everyone's configurations. See https://checkstyle.org/writingchecks.html#Defining_Check_Properties |
Understood. Now clarify one thing - What should be returned for the non-existent field checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/site/SiteUtil.java Lines 278 to 288 in a6a339f Since |
|
IMO, most likely there shouldn't be a change in that method as it should be kept simple and to one point. I would like at |
|
See the non-existent fields are taken care of in this class (SiteUtil) only for checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/site/SiteUtil.java Lines 1001 to 1049 in a6a339f That's why I was asking how to tackle that for |
|
What value be expected if it isn't a field rather a setter? Inside, getFieldValue, field.get(instance); I am struggling to do that for |
Getters have no connection to properties and how Checkstyle works with them. They can technically be whatever they want, even be named wrongly, though I am not sure that is the case with your example. Truthfully, our property process only looks at setters. Fields are just storage containers, and don't need to be there, or even named after the setter. We continue that since it is what makes sense for Java and bean classes.
I don't pretend to know the macro's logic at a detailed level, which is why I can't provide any guidance as you are expecting exact instructions. I recommend instead to examine the full code and find what logically makes sense. Not everything is going to be cut and dry. If you are unable to identify what to fix, then I would probably recommend an easier issue.
As I mentioned before, the macro should be compared to
Maybe look into this field and why it can't be used or isn't working correctly. |
It is not being used because it has |
|
@relentless-pursuit Exactly this is my observation! On correcting the class name to SuppressWarningsHolder So we need to handle this NullPointerException...! Raising a PR on this soon. |
|
I have raised a PR on this. Some test cases are failing, which can be corrected as we proceed with review. It would be great if i can have a review on the core logic where i used CHECK_ALIAS_MAP stores aliasList values. |
…cription how we interpretate value
…cription how we interpretate value
…cription how we interpretate value
…cription how we interpretate value
…mport Issue checkstyle#13839: make aliasList simple String[] but extend description how we interpretate value minor: Remove closed issue reference Issue checkstyle#14042: avoid class file clash on case-insensitive filesystem dependency: bump org.apache.maven.plugins:maven-project-info-reports-plugin Bumps [org.apache.maven.plugins:maven-project-info-reports-plugin](https://github.com/apache/maven-project-info-reports-plugin) from 3.4.5 to 3.5.0. - [Commits](apache/maven-project-info-reports-plugin@maven-project-info-reports-plugin-3.4.5...maven-project-info-reports-plugin-3.5.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-project-info-reports-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…mport Issue checkstyle#13839: make aliasList simple String[] but extend description how we interpretate value minor: Remove closed issue reference Issue checkstyle#14042: avoid class file clash on case-insensitive filesystem dependency: bump org.apache.maven.plugins:maven-project-info-reports-plugin Bumps [org.apache.maven.plugins:maven-project-info-reports-plugin](https://github.com/apache/maven-project-info-reports-plugin) from 3.4.5 to 3.5.0. - [Commits](apache/maven-project-info-reports-plugin@maven-project-info-reports-plugin-3.4.5...maven-project-info-reports-plugin-3.5.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-project-info-reports-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…mport Issue checkstyle#13839: make aliasList simple String[] but extend description how we interpretate value minor: Remove closed issue reference Issue checkstyle#14042: avoid class file clash on case-insensitive filesystem dependency: bump org.apache.maven.plugins:maven-project-info-reports-plugin Bumps [org.apache.maven.plugins:maven-project-info-reports-plugin](https://github.com/apache/maven-project-info-reports-plugin) from 3.4.5 to 3.5.0. - [Commits](apache/maven-project-info-reports-plugin@maven-project-info-reports-plugin-3.4.5...maven-project-info-reports-plugin-3.5.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-project-info-reports-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Revert "Issue checkstyle#4845: Removed Checkstyle Dependency For CustomOrderImport" This reverts commit 6482222.
there is problem to migrate SuppressWarningsHolder to property macro for
aliasListforpublic void setAliasList(String... aliasList) {migration:
failure:
The text was updated successfully, but these errors were encountered: