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

Issue #4437: Modified UnusedImportsCheckTest.java and moved its input files to the unusedimports subdirectory #4470

Merged
merged 1 commit into from
Jun 20, 2017

Conversation

subkrish
Copy link
Contributor

Issue #4437

This PR moves all inputs of UnusedImportsCheckTest of the imports package to a new subdirectory 'unusedimports'.

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnveach , I found only one problem, please do review.

@subkrish , please fix my point:

}

@Test
public void testAnnotations() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(UnusedImportsCheck.class);
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
verify(checkConfig, getNonCompilablePath("package-info.java"), expected);
verify(checkConfig, getNonCompilablePath("InputUnusedImportsAnnotations.java"), expected);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please restore name of the file - it is special file.
https://www.intertech.com/Blog/whats-package-info-java-for/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romani It is still an input file. If test doesn't rely on requiring a specific file name, why must we keep it like that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, lets keep new name. Checkstyle do not care, it was just signal to used that file was weird because it is non java file actually.
package-info.java - is parse-able by us, that is why we process it. In other words , it does not make problems for us so we work with it.
module-info.java - is in the same family of file in java project. We do not process it, because it completely different by structure and .... .

so, to make it clear for future contributor, lets change comment content from Annotated package definition to check if import is declared unused --> this file content is based on package-info.java file structure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

@rnveach rnveach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only item I saw.

import java.lang.String;
import static java.lang.Math.PI;

public class InputRedundantImport_UnnamedPackage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class name was not changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

… its input files to the unusedimports subdirectory
@subkrish subkrish force-pushed the fix-issue-4437-unusedimports branch from a419328 to 131ea14 Compare June 20, 2017 01:10
@rnveach rnveach merged commit d1ecad1 into checkstyle:master Jun 20, 2017
@subkrish subkrish deleted the fix-issue-4437-unusedimports branch June 20, 2017 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants