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

3rd party root modules not finding by simple name #4417

Closed
rnveach opened this Issue Jun 4, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@rnveach
Member

rnveach commented Jun 4, 2017

$ javac -cp "checkstyle-7.8-all.jar" com/puppycrawl/tools/checkstyle/TestRootModule.java

$ cat com/puppycrawl/tools/checkstyle/TestRootModule.java
package com.puppycrawl.tools.checkstyle;

public class TestRootModule extends Checker {
}

$ jar -cf MyCustom.jar com/puppycrawl/tools/checkstyle/TestRootModule.class

$ javac TestClass.java

$ cat TestClass.java
public class TestClass {
    void method() {
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="TestRootModule">
    <property name="charset" value="UTF-8"/>
</module>

$ java -classpath MyCustom.jar;checkstyle-7.8-all.jar com.puppycrawl.tools.checkstyle.Main -c TestConfig.xml TestClass.java
com.puppycrawl.tools.checkstyle.api.CheckstyleException: Unable to instantiate 'TestRootModule' class, it is also not possible to instantiate it as com.puppycrawl.tools.checkstyle..TestRootModule, TestRootModuleCheck, com.puppycrawl.tools.checkstyle..TestRootModuleCheck. Please recheck that class name is specified as canonical name or read how to configure short name usage http://checkstyle.sourceforge.net/config.html#Packages. Please also recheck that provided ClassLoader to Checker is configured correctly.
    at com.puppycrawl.tools.checkstyle.PackageObjectFactory.createModule(PackageObjectFactory.java:166)
    at com.puppycrawl.tools.checkstyle.Main.getRootModule(Main.java:449)
    at com.puppycrawl.tools.checkstyle.Main.runCheckstyle(Main.java:416)
    at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:359)
    at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)
Checkstyle ends with 1 errors.

Make key note to the packages it is trying to instantiate them by. They all have double periods. This is preventing custom root modules from being instantiated.

Checkstyle 7.7 doesn't have double periods and works fine. This was most likely broken by Issue #3607 .

$ java -classpath MyCustom.jar;checkstyle-7.7-all.jar com.puppycrawl.tools.checkstyle.Main -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

Our test didn't pick this up because it uses a full class path to load the module.

As a side note, the list of packages it tries for the check is very limited, most likely because it doesn't even load the list from the checkstyle_packages.xml file.

@rnveach rnveach changed the title from 3rd party root modules broken to 3rd party root modules not finding by simple name Jun 4, 2017

@romani romani added the approved label Jun 4, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 4, 2017

Member

@rnveach , please share links to TestRootModule , goal of CLI usage in issue reporting is to reproduce issue easily and without problems. Right now your test case can not be reproduced easily, extra jar in -cp .... should be referenced.

Our test didn't pick this up because

please point to code of UT, we will extend UT in scope of this issue.

Member

romani commented Jun 4, 2017

@rnveach , please share links to TestRootModule , goal of CLI usage in issue reporting is to reproduce issue easily and without problems. Right now your test case can not be reproduced easily, extra jar in -cp .... should be referenced.

Our test didn't pick this up because

please point to code of UT, we will extend UT in scope of this issue.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jun 4, 2017

Member

please point to code of UT

Added to first post.

please share links to TestRootModule

Name of module was just example. This is why I used an exception error to show the problem.
RootModule that I found error with is in my branch embeded into Checkstyle so there is no extra JAR needed.
Exception is as it appears with released checkstyle which shows the double period problem.
I will edit the post for a dummy example.

Member

rnveach commented Jun 4, 2017

please point to code of UT

Added to first post.

please share links to TestRootModule

Name of module was just example. This is why I used an exception error to show the problem.
RootModule that I found error with is in my branch embeded into Checkstyle so there is no extra JAR needed.
Exception is as it appears with released checkstyle which shows the double period problem.
I will edit the post for a dummy example.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jun 5, 2017

Member

@romani I updated first post with compiling results, jar packaging, and 7.7 command and results.

Member

rnveach commented Jun 5, 2017

@romani I updated first post with compiling results, jar packaging, and 7.7 command and results.

rnveach added a commit to rnveach/checkstyle that referenced this issue Jun 9, 2017

romani added a commit that referenced this issue Jun 12, 2017

@romani romani added the bug label Jun 12, 2017

@romani romani added this to the 7.8.2 milestone Jun 12, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 12, 2017

Member

fix is merged.

Member

romani commented Jun 12, 2017

fix is merged.

@romani romani closed this Jun 12, 2017

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