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 #213: update to CS 8.20 #216

Merged
merged 1 commit into from
Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Compatibility matrix from checkstyle team:

Checkstyle Plugin|Sonar min|Sonar max|Checkstyle|Jdk
-----------------|---------|---------|----------|---
4.20|6.7 |7.7+|8.20|1.8
4.19|6.7 |7.7+|8.19|1.8
4.18|6.7 |7.7+|8.18|1.8
4.17|6.7 |7.5|8.17|1.8
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
</ciManagement>

<properties>
<checkstyle.version>8.19</checkstyle.version>
<checkstyle.version>8.20</checkstyle.version>
<sonar.version>6.7</sonar.version>
<sonar-java.version>5.12.0.17701</sonar-java.version>
<maven.checkstyle.plugin.version>2.17</maven.checkstyle.plugin.version>
Expand Down
13 changes: 13 additions & 0 deletions src/main/resources/com/sonar/sqale/checkstyle-model.xml
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,19 @@
<txt>min</txt>
</prop>
</chc>
<chc>
<rule-repo>checkstyle</rule-repo>
<rule-key>com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck</rule-key>
Copy link
Member

Choose a reason for hiding this comment

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

@muhlba91 Does every module need to be listed here?
It doesn't seem to be part of our test to verify everything is listed in here. https://github.com/checkstyle/sonar-checkstyle/blob/master/src/test/java/org/sonar/plugins/checkstyle/internal/ChecksTest.java#L87-L94

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't use SQALE on my instances but I don't think it's necessary since, afaik, if it's not listed iit doesn't have an impact on the score?
However, I added it because there were other Javadoc checks present and I kept it on-par with them. Do we have any rules for the SQALE stuff?

Anyhow, how is SQALE the SQALE support in SonarQube atm? At least in the API I read that it got deprecated:

Copy link
Member

@rnveach rnveach May 10, 2019

Choose a reason for hiding this comment

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

This is my first time seeing it so I don't believe there are any rules. I don't know what SQALE is so I couldn't identify what needs to be done with it.
Unless you or @romani know more, I suggest we just create an issue for a test to verify it has valid information and not require checks be listed.

Edit:
From looking online, it looks like it is just listing what debt the check creates when a violation is found. I think @romani had some hesitation on this since it is subjective. It couldn't hurt to list every check for another issue but I am not sure we would really look into the values suggested.

Copy link
Member

Choose a reason for hiding this comment

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

this config came from Sonarqube team.
I never understand how to it is possible to estimate fix in time, in most cases it does work at all.
Such estimations NEVER worked well, it was only source of laugh ..... ones you spend a day with "10 min fix".
If Sonar do not demand such config be for each rule - I would be happy to remove them completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ok, I'll see if Sonar is happy when the file is empty/removed. I think the listed checks are anyways not up to date with all checks available.
If so, we can remove it and also don't have to copy over the SQALE source code from sslr. Will create an issue on that latest tomorrow. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

created #219

<prop>
<key>remediationFunction</key>
<txt>CONSTANT_ISSUE</txt>
</prop>
<prop>
<key>offset</key>
<val>10</val>
<txt>min</txt>
</prop>
</chc>
<chc>
<rule-repo>checkstyle</rule-repo>
<rule-key>com.puppycrawl.tools.checkstyle.checks.design.FinalClassCheck</rule-key>
Expand Down
5 changes: 5 additions & 0 deletions src/main/resources/org/sonar/l10n/checkstyle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocVariableCh
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocVariableCheck.param.scope=visibility scope where Javadoc comments are checked
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocVariableCheck.param.ignoreNamePattern=regexp to define variable names to ignore
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocVariableCheck.param.tokens=tokens to check
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck.name=Missing Javadoc Type
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck.param.excludeScope=visibility scope where Javadoc comments are not checked
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck.param.scope=visibility scope where Javadoc comments are checked
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck.param.skipAnnotations=specify the list of annotations that allow missed documentation. Only short names are allowed, e.g. Generated
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck.param.tokens=tokens to check
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.coding.IllegalInstantiationCheck.name=Illegal Instantiation
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.coding.IllegalInstantiationCheck.param.classes=classes that should not be instantiated
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.coding.IllegalInstantiationCheck.param.tokens=tokens to check
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Checks for missing Javadoc comments for class, enum, interface, and annotation interface definitions. The scope to verify is specified using the Scope class and defaults to Scope.PUBLIC. To verify another scope, set property scope to one of the Scope constants.
18 changes: 18 additions & 0 deletions src/main/resources/org/sonar/plugins/checkstyle/rules.xml
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,24 @@
<status>READY</status>
</rule>

<rule key="com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck">
<priority>MAJOR</priority>
<name><![CDATA[Missing Javadoc Type]]></name>
<configKey><![CDATA[Checker/TreeWalker/MissingJavadocType]]></configKey>
<cardinality>MULTIPLE</cardinality>
<param key="scope" type="s[nothing,public,protected,package,private,anoninner]">
<defaultValue>public</defaultValue>
</param>
<param key="excludeScope" type="s[nothing,public,protected,package,private,anoninner]">
</param>
<param key="skipAnnotations" type="s{}">
</param>
<param key="tokens" type="s[INTERFACE_DEF,CLASS_DEF,ENUM_DEF,ANNOTATION_DEF]">
<defaultValue>INTERFACE_DEF,CLASS_DEF,ENUM_DEF,ANNOTATION_DEF</defaultValue>
</param>
<status>READY</status>
</rule>

<rule key="com.puppycrawl.tools.checkstyle.checks.blocks.LeftCurlyCheck">
<priority>MINOR</priority>
<name><![CDATA[Left Curly]]></name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void test() {
assertThat(repository.language()).isEqualTo("java");

final List<RulesDefinition.Rule> rules = repository.rules();
assertThat(rules).hasSize(157);
assertThat(rules).hasSize(158);

for (RulesDefinition.Rule rule : rules) {
assertThat(rule.key()).isNotNull();
Expand Down