Skip to content
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

Disallowing user to use incomplete fully qualified Check names in config file #4456

Closed
romani opened this issue Jun 14, 2017 · 1 comment
Closed

Comments

@romani
Copy link
Member

romani commented Jun 14, 2017

base on #4414 , #4387

code quality question was raised at #4430 (review) , see discussion from thsi comment and down for details.

What to do:

  1. no changes to functionality is expected , except for disallowing to user use names like com.puppycrawl.tools.checkstyle.checks.naming.ConstantName, if user use fully qualified name, it should be absolutely correct ! Expected fully qualified Check name is com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck

  2. Additionally there should refactoring to avoid logic that is based on catching reflection exceptions.
    We do have all information in collection like thirdPartyNameToFullModuleNames and NAME_TO_FULL_MODULE_NAME (at keys and values). So we could predict cases where will not have success to load class by reflection at createObject(code)

It is expected to have code like below(deliver to user all possible classpath issues):

    private Object createObject(String className) throws CheckstyleException {
        Object instance = null;
            try {
               Class<?> clazz = Class.forName(className, true, moduleClassLoader);

                final Constructor<?> declaredConstructor = clazz.getDeclaredConstructor();
                declaredConstructor.setAccessible(true);
                instance = declaredConstructor.newInstance();
            }
            catch (final ReflectiveOperationException  | NoClassDefFoundError  ex) {
                throw new CheckstyleException("Unable to instantiate " + className, ex);
            }
        return instance;
}

code:


        if (instance == null) {
            instance = createObject(name);
        }
        final String nameCheck = name + CHECK_SUFFIX;
        if (instance == null) {
            instance = createObject(nameCheck);
        }
        if (instance == null) {
 .....

should be extended to validate collection and call createObject only when we sure about success to load class.

@romani romani added the bug label Jun 14, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Jul 15, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Aug 14, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 25, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 25, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Sep 25, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 3, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 4, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 10, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 12, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 12, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 12, 2017
djydewang added a commit to djydewang/checkstyle that referenced this issue Oct 23, 2017
romani pushed a commit that referenced this issue Oct 28, 2017
@romani romani added this to the 8.4 milestone Oct 28, 2017
@romani
Copy link
Member Author

romani commented Oct 28, 2017

@djydewang , thanks a lot for elegant fix!

@romani romani closed this as completed Oct 28, 2017
romani added a commit to sevntu-checkstyle/checkstyle-samples that referenced this issue Oct 29, 2017
romani added a commit to sevntu-checkstyle/sevntu.checkstyle that referenced this issue Oct 29, 2017
romani added a commit to sevntu-checkstyle/sevntu.checkstyle that referenced this issue Oct 29, 2017
timurt pushed a commit to timurt/checkstyle that referenced this issue Dec 19, 2017
kariem pushed a commit to kariem/sevntu.checkstyle that referenced this issue Jul 26, 2018
kariem pushed a commit to kariem/sevntu.checkstyle that referenced this issue Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant