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

TranslationCheck reports duplicate lines in xml report #5103

Closed
Nimfadora opened this issue Sep 10, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@Nimfadora
Copy link
Contributor

commented Sep 10, 2017

taken from #5043

When checking several files with TranslationCheck and generating xml output, check reports duplicate lines with default translation file. Moreover, on different operation systems this line appears in different places.

steps to reproduce:

  1. generate properties files with some error, for e.z.:
$ cat props/InputTranslationCheckFireErrors.properties 
someKey=Some key
anotherKey=one more

and

$ cat props/InputTranslationCheckFireErrors_de.properties 
someKey=einige Schlüssel
  1. generate checkstyle profile, so that there was some absent translation:
<?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 = "Checker">
    <module name="Translation">
        <property name="requiredTranslations" value="de, ja"/>
        <property name="baseName" value="^InputTranslation.*$"/>
    </module>
</module>
  1. generate all-jar and run:
    $ java -jar checkstyle-8.3-SNAPSHOT-all.jar -f xml -c /home/valeria/IdeaProjects/contribution/checkstyle-tester/my_check.xml /home/valeria/IdeaProjects/contribution/checkstyle-tester/props/

  2. In report you will see duplicate line:

    <file name="/home/valeria/IdeaProjects/contribution/checkstyle-tester/props/InputTranslationCheckFireErrors.properties">
    </file>

On windows and linux you will receive such report:

<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="8.3-SNAPSHOT">
    <file name="/home/valeria/IdeaProjects/contribution/checkstyle-tester/props/InputTranslationCheckFireErrors.properties">
    </file>
    <file name="/home/valeria/IdeaProjects/contribution/checkstyle-tester/props/InputTranslationCheckFireErrors_de.properties">
    </file>
    <file name="/home/valeria/IdeaProjects/contribution/checkstyle-tester/props">
        <error line="0" severity="warning" message="Properties file &apos;InputTranslationCheckFireErrors_ja.properties&apos; is missing." source="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>
    </file>
    <file name="/home/valeria/IdeaProjects/contribution/checkstyle-tester/props/InputTranslationCheckFireErrors_de.properties">
        <error line="0" severity="warning" message="Key &apos;anotherKey&apos; missing." source="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>
    </file>
    <file name="/home/valeria/IdeaProjects/contribution/checkstyle-tester/props/InputTranslationCheckFireErrors.properties">
    </file>
</checkstyle>

on Mac OS line will be in different place:

<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="8.3-SNAPSHOT">
    <file name="/home/valeria/IdeaProjects/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/misc/translation/InputTranslationCheckFireErrors.properties">
    </file>
    <file name="/home/valeria/IdeaProjects/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/misc/translation/InputTranslationCheckFireErrors_de.properties">
    </file>
    <file name="/home/valeria/IdeaProjects/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/misc/translation">
        <error line="0" severity="error" message="Properties file &apos;InputTranslationCheckFireErrors_ja.properties&apos; is missing." source="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>
    </file>
    <file name="/home/valeria/IdeaProjects/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/misc/translation/InputTranslationCheckFireErrors.properties">
    </file>
    <file name="/home/valeria/IdeaProjects/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/misc/translation/InputTranslationCheckFireErrors_de.properties">
        <error line="0" severity="error" message="Key &apos;anotherKey&apos; missing." source="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>
    </file>
</checkstyle>
@romani

This comment has been minimized.

Copy link
Member

commented Sep 12, 2017

@Nimfadora , it is not good to open issue with usage on SANPSHOT version, it is unclear how to reproduce. If you can not reproduce on release version you need to specify on which commit to build SNAPSHOT version.

Here is my steps to reproduce and it looks like problem exists even before your changes in 8.2 release.

/var/tmp$ tree props/
props/
├── InputTranslationCheckFireErrors_de.properties
└── InputTranslationCheckFireErrors.properties

0 directories, 2 files

$ java -jar checkstyle-8.2-all.jar -c config.xml -f xml props/ 
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="8.2">
<file name="/var/tmp/props/InputTranslationCheckFireErrors.properties">
</file>
<file name="/var/tmp/props/InputTranslationCheckFireErrors_de.properties">
</file>
<file name="/var/tmp/props">
<error line="0" severity="error" message="Properties file &apos;InputTranslationCheckFireErrors_ja.properties&apos; is missing." source="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>
</file>
<file name="props/InputTranslationCheckFireErrors_de.properties">
<error line="0" severity="error" message="Key &apos;anotherKey&apos; missing." source="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>
</file>
<file name="props/InputTranslationCheckFireErrors.properties">
</file>
</checkstyle>
Checkstyle ends with 2 errors.

if that is true, please update description to use 8.2 release binaries and short name of path.
Attention to my report, looks like duplicates are from different paths (absolute vs relative).

@rnveach

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Duplicate lines are due to multi-file validation.
See https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java#L459-L467
File must be triggered as started before printing violations for each as we are in the finishProcessing phase of the check and files are not normally processed here.

@romani We can check if violations are present before we send the file started event.
If you agree please approve issue.

@rnveach

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

We cannot limit excess file reporting in multi-file mode. Multi-file check can produce violation on any file it has already processed once during finishProcessing. We can limit reporting multi-files with no violations, but we can't merge it with the first reporting if there are violations.

different paths (absolute vs relative).

This is because we call getPath on the file instead of getAbsolutePath. Translation is creating the file name itself.

@rnveach rnveach added the approved label Apr 6, 2019

@rnveach

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

Moreover, on different operation systems this line appears in different places.

This is because we deal fully with Sets without any hash implementation. filesAssociatedWithKeys is a hash map so there is no guarantee of order in this situation and we prefer it that way.
Tests were already fixed to accommodate this.

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 6, 2019

romani added a commit that referenced this issue Apr 17, 2019

@romani romani added the bug label Apr 17, 2019

@romani romani added this to the 8.20 milestone Apr 17, 2019

@rnveach

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Fix was merged

@rnveach rnveach closed this Apr 17, 2019

peterdemaeyer added a commit to peterdemaeyer/checkstyle that referenced this issue Apr 27, 2019

peterdemaeyer added a commit to peterdemaeyer/checkstyle that referenced this issue Apr 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.