New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand eclipse compiler to check test code #5116

Closed
rnveach opened this Issue Sep 15, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@rnveach
Member

rnveach commented Sep 15, 2017

Identified at #5113 ,

When I enable these settings in Eclipse IDE, they automatically apply them to testso I still see errors in our configuration. I think we should update our CI to include test. This results in 24 errors, most of which need to just be suppressed as they are ways of testing.

Here is the list:

----------
1. INFO in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractTypeAwareCheckTest.java (at line 40)
    @SuppressWarnings("deprecation")
                      ^^^^^^^^^^^^^
At least one of the problems in category 'deprecation' is not analysed due to a compiler option being ignored
----------
----------
2. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/filters/XpathFilterTest.java (at line 122)
    new XpathFilter("InputXpathFilterSuppressByXpath", "Test", null, xpath);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
----------
3. INFO in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java (at line 586)
    @SuppressWarnings("deprecation")
                      ^^^^^^^^^^^^^
At least one of the problems in category 'deprecation' is not analysed due to a compiler option being ignored
----------
----------
4. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PropertyCacheFileTest.java (at line 88)
    new PropertyCacheFile(null, "");
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
5. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PropertyCacheFileTest.java (at line 96)
    new PropertyCacheFile(config, null);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
----------
6. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PropertiesExpanderTest.java (at line 32)
    new PropertiesExpander(null);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
----------
7. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java (at line 72)
    new PackageObjectFactory(new HashSet<>(), null);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
8. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java (at line 83)
    new PackageObjectFactory("test", null);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
9. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java (at line 95)
    new PackageObjectFactory(Collections.singleton(null), classLoader);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
10. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java (at line 107)
    new PackageObjectFactory((String) null, classLoader);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
11. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java (at line 134)
    public void testCreateModuleWithNonExistName() throws CheckstyleException {
                                                          ^^^^^^^^^^^^^^^^^^^
The declared exception CheckstyleException is not actually thrown by the method testCreateModuleWithNonExistName() from type PackageObjectFactoryTest
----------
----------
12. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/api/AutomaticBeanTest.java (at line 205)
    public void doSmth() {
                ^^^^^^^^
The method doSmth() from the type AutomaticBeanTest.TestBean is never used locally
----------
----------
13. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractFileSetCheckTest.java (at line 77)
    check.setFileExtensions(null);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Type null of the last argument to method setFileExtensions(String...) doesn't exactly match the vararg parameter type. Cast to String[] to confirm the non-varargs invocation, or pass individual arguments of type String for a varargs invocation.
----------
----------
14. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/api/FileTextTest.java (at line 56)
    new FileText(new File("any name"), charsetName);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
----------
15. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/api/SeverityLevelCounterTest.java (at line 32)
    new SeverityLevelCounter(null);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
----------
16. INFO in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/api/FileContentsTest.java (at line 42)
    @SuppressWarnings("deprecation")
                      ^^^^^^^^^^^^^
At least one of the problems in category 'deprecation' is not analysed due to a compiler option being ignored
----------
----------
17. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/utils/ModuleReflectionUtilsTest.java (at line 141)
    protected ValidCheckstyleClass() {
              ^^^^^^^^^^^^^^^^^^^^^^
The constructor ModuleReflectionUtilsTest.ValidCheckstyleClass() is never used locally
----------
18. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/utils/ModuleReflectionUtilsTest.java (at line 147)
    protected InvalidNonAutomaticBeanClass() {
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The constructor ModuleReflectionUtilsTest.InvalidNonAutomaticBeanClass() is never used locally
----------
19. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/utils/ModuleReflectionUtilsTest.java (at line 259)
    protected NotCheckstyleCheck() {
              ^^^^^^^^^^^^^^^^^^^^
The constructor ModuleReflectionUtilsTest.NotCheckstyleCheck() is never used locally
----------
20. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/utils/ModuleReflectionUtilsTest.java (at line 265)
    private int field;
                ^^^^^
The value of the field ModuleReflectionUtilsTest.InvalidNonDefaultConstructorClass.field is not used
----------
21. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/utils/ModuleReflectionUtilsTest.java (at line 267)
    protected InvalidNonDefaultConstructorClass(int data) {
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The constructor ModuleReflectionUtilsTest.InvalidNonDefaultConstructorClass(int) is never used locally
----------
----------
22. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/doclets/TokenTypesDocletTest.java (at line 108)
    new TestMessager(context);
    ^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
23. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/doclets/TokenTypesDocletTest.java (at line 124)
    new TestMessager(context);
    ^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
24. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/doclets/TokenTypesDocletTest.java (at line 148)
    new TestMessager(context);
    ^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
25. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/doclets/TokenTypesDocletTest.java (at line 170)
    new TestMessager(context);
    ^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
----------
26. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/XMLLoggerTest.java (at line 56)
    new XMLLogger(outStream, false);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
27. ERROR in /checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/XMLLoggerTest.java (at line 79)
    new XMLLogger(outStream, false);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The allocated object is never used
----------
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 16, 2017

Member

most of which need to just be suppressed as they are ways of testing.

Eclipse have now suppression mode except for by annotations in code.
I tried to keep code clean from such model as much as possible.
Do we have an way to reconsile such requests ?

Member

romani commented Sep 16, 2017

most of which need to just be suppressed as they are ways of testing.

Eclipse have now suppression mode except for by annotations in code.
I tried to keep code clean from such model as much as possible.
Do we have an way to reconsile such requests ?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Sep 16, 2017

Member

Do we have an way to reconsile such requests ?

Like I said, as it is right now with checkstyle's eclipse file in the IDE, I am getting errors all over the test code. Eclipse doesn't have an ability to suppress things another way unless they added something in newer IDEs. You also can't separate out different org.eclipse.jdt.core.prefs between folders, only different projects.
In it's view, if it is an error it must be fixed and you can't suppress it at all. If we don't want to suppress, we could change them to warnings and have our custom script only enforce warnings be fixed on main code and either ignore all of test's non-errors maybe via -nowarn: or some custom way.

The allocated object is never used

Another idea is to fool it's validation by using reflection to call the constructor just for test so we are hiding that the allocation occurs.
Then this just leaves us the never used errors which might be fixable.

Member

rnveach commented Sep 16, 2017

Do we have an way to reconsile such requests ?

Like I said, as it is right now with checkstyle's eclipse file in the IDE, I am getting errors all over the test code. Eclipse doesn't have an ability to suppress things another way unless they added something in newer IDEs. You also can't separate out different org.eclipse.jdt.core.prefs between folders, only different projects.
In it's view, if it is an error it must be fixed and you can't suppress it at all. If we don't want to suppress, we could change them to warnings and have our custom script only enforce warnings be fixed on main code and either ignore all of test's non-errors maybe via -nowarn: or some custom way.

The allocated object is never used

Another idea is to fool it's validation by using reflection to call the constructor just for test so we are hiding that the allocation occurs.
Then this just leaves us the never used errors which might be fixable.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 16, 2017

Member

I am getting errors all over the test code

please send fix to "ignore" such rules to let this actually work, the same as in Eclipse - it is critical point.

we could change them to warnings

I personally do not like some violations that are not critical to fix to pass build, nobody fix them and they only accumulate and pollute code, hide other problems .... .
Lets put ignore on them for now. And keep Eclipse clean from any warnings.

either ignore all of test's non-errors maybe via -nowarn: or some custom way

if Eclipse does not have any other suppression, I have never seen any except for annotation. .... I rejected to all static analysis tool to use annotations, even for IDEA, not sure we can allow this now.

Member

romani commented Sep 16, 2017

I am getting errors all over the test code

please send fix to "ignore" such rules to let this actually work, the same as in Eclipse - it is critical point.

we could change them to warnings

I personally do not like some violations that are not critical to fix to pass build, nobody fix them and they only accumulate and pollute code, hide other problems .... .
Lets put ignore on them for now. And keep Eclipse clean from any warnings.

either ignore all of test's non-errors maybe via -nowarn: or some custom way

if Eclipse does not have any other suppression, I have never seen any except for annotation. .... I rejected to all static analysis tool to use annotations, even for IDEA, not sure we can allow this now.

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 24, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 24, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 24, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 24, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 24, 2017

@romani romani added the approved label Sep 25, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 25, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Sep 25, 2017

rnveach added a commit that referenced this issue Sep 25, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Sep 25, 2017

Member

Fix was merged

Member

rnveach commented Sep 25, 2017

Fix was merged

@rnveach rnveach closed this Sep 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment