Skip to content

Config: use new treewalker property to skip non-compiled files in regression#877

Merged
rnveach merged 1 commit intocheckstyle:masterfrom
checkstyle-GSoC25:new-property-in-config
Jul 13, 2024
Merged

Config: use new treewalker property to skip non-compiled files in regression#877
rnveach merged 1 commit intocheckstyle:masterfrom
checkstyle-GSoC25:new-property-in-config

Conversation

@mahfouz72
Copy link
Member

@mahfouz72 mahfouz72 force-pushed the new-property-in-config branch 2 times, most recently from 1ae300d to 56df849 Compare July 13, 2024 01:46
@mahfouz72
Copy link
Member Author

@romani ping please can you help to fix CI

failed during checkstyle configuration: Property 'skipFileOnJavaParseException' does not exist, please check the documentation. Do we need to get a release first to detect new properties?

@romani romani requested a review from nrmancuso July 13, 2024 03:33
@rnveach rnveach force-pushed the new-property-in-config branch from 56df849 to 9f3c0b0 Compare July 13, 2024 03:53
@rnveach
Copy link
Contributor

rnveach commented Jul 13, 2024

https://app.circleci.com/pipelines/github/checkstyle/contribution/357/workflows/3b96ba65-6f9f-4f5a-aeaf-0685f220ed0f/jobs/2476

com.puppycrawl.tools.checkstyle.api.CheckstyleException: Property 'skipFileOnJavaParseException' does not exist, please check the documentation

@mahfouz72 This is not a Checker property.

Copy link
Contributor

@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.

@mahfouz72 mahfouz72 force-pushed the new-property-in-config branch from 9f3c0b0 to 438a24b Compare July 13, 2024 10:16
@mahfouz72
Copy link
Member Author

@rnveach done

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.

Item

</module>

<!-- as we run on regression even on non-compiled files we need to skip exceptions on them -->
<property name="skipFileOnJavaParseException" value="true"/>
Copy link
Member

Choose a reason for hiding this comment

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

Please put them under other Checker properties. Group them together above , before first inner module

Copy link
Member Author

Choose a reason for hiding this comment

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

Please put them under other Checker properties.

This is not checker property. Do you mean at the first line in tree walker module? like this

    <module name="TreeWalker">
         <!-- as we run on regression even on non-compiled files we need to skip exceptions on them -->
         <property name="skipFileOnJavaParseException" value="true"/>
         <property name="javaParseExceptionSeverity" value="ignore"/>

         <!-- Example of checkstyle Check usage -->
         <!-- PLEASE CHANGE IT TO CHECK YOU ARE TESTING !!!! -->
         <module name="ThrowsCount">
             <property name="max" value="20000000"/>
         </module>

         <!-- Example of sevntu.checkstyle Check usage -->
         <!-- <module name="NestedSwitchCheck"/> -->

         <!-- suppress javadoc parsing errors, as we test Check not a parser -->
         <module name="SuppressionXpathSingleFilter">
            <property name="message" value="Javadoc comment at column \d+ has parse error"/>
         </module>
    </module>

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to put it after the filter and leave the check modules above, right after <module name="TreeWalker">

Copy link
Member

@romani romani Jul 13, 2024

Choose a reason for hiding this comment

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

Treewalker, yes, all propertties are on most top before any inner modules.

 <module name="TreeWalker">
         <!-- as we run on regression even on non-compiled files we need to skip exceptions on them -->
         <property name="skipFileOnJavaParseException" value="true"/>
         <property name="javaParseExceptionSeverity" value="ignore"/>

I insisting on this format.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, done

<!-- <module name="NestedSwitchCheck"/> -->

<!-- usuppress javadoc parsing errors, as we test Check not a parser -->
<!-- suppress javadoc parsing errors, as we test Check not a parser -->
Copy link
Contributor

Choose a reason for hiding this comment

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

This has been killing me for years, thank you for fixing this.

@nrmancuso nrmancuso assigned romani and unassigned nrmancuso Jul 13, 2024
@romani
Copy link
Member

romani commented Jul 13, 2024

@relentless-pursuit , please subscribe to this PR, as we merge please update test-configs repo with new content of template config.

@mahfouz72 mahfouz72 force-pushed the new-property-in-config branch 2 times, most recently from f910de5 to 70f2f5b Compare July 13, 2024 14:57
@romani romani assigned rnveach and unassigned romani Jul 13, 2024
@rnveach rnveach merged commit 45007b9 into checkstyle:master Jul 13, 2024
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.

4 participants