Issue #16378: Validate that all Inputs mentioned all default properties by Anmol202005 · Pull Request #16760 · checkstyle/checkstyle

Anmol202005 · 2025-04-04T20:04:54Z

updated InlineConfigParser.java to validate that all properties are specified in Input file. if all properties are default, all properties should be mentioned in Input file and all has prefix (default).

Pankraz76

well done

Anmol202005 · 2025-04-06T09:30:14Z

@pankratz76 will get started with beautification once the CI is green.

rdiachenko

@Anmol202005 this PR is huge and hard to review properly. Could you think of a way to split it onto smaller ones. Can we focus on updating the inline parser first and applying its validation to a single test only, and after that gradually update all the rest test classes?

Anmol202005 · 2025-04-06T16:09:02Z

@Anmol202005 this PR is huge and hard to review properly. Could you think of a way to split it onto smaller ones.

agreed, what about if we raise a supplemental PR with all changes in input file and update the inline parser in this PR :)

rdiachenko · 2025-04-06T21:05:13Z

@Anmol202005 this PR is huge and hard to review properly. Could you think of a way to split it onto smaller ones.

agreed, what about if we raise a supplemental PR with all changes in input file and update the inline parser in this PR :)

Let's update inline parser + one test file in this PR. In the follow-up PRs let's do one test package per PR. Please create a list of subtasks in the issue for each package so we can track what has been fixed.

Anmol202005 · 2025-04-08T04:57:51Z

Let's update inline parser + one test file in this PR. In the follow-up PRs let's do one test package per PR. Please create a list of subtasks in the issue for each package so we can track what has been fixed.

look a better approach!
meanwhile i just updated all the input Files , will revert them :(

Pankraz76 · 2025-04-08T09:27:34Z

Let's update inline parser + one test file in this PR. In the follow-up PRs let's do one test package per PR. Please create a list of subtasks in the issue for each package so we can track what has been fixed.

look a better approach! meanwhile i just updated all the input Files , will revert them :(

maybe keep them locally in a dedicated branch, so you can reuse later and try to merge after the supplemental PR is in main.

Anmol202005 · 2025-04-09T16:51:20Z

@rdiachenko,
updated the pr as discussed, only finalparameters package is updated.
raised issue for the rest: #16807

rdiachenko · 2025-04-09T20:43:35Z

@rdiachenko, updated the pr as discussed, only finalparameters package is updated. raised issue for the rest: #16807

Thanks, @Anmol202005 . Please make use ci is green, there are failed inspections like this one:

<problem>
<file>file://$PROJECT_DIR$/src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java</file>
<line>958</line>
<module>project</module>
<package>com.puppycrawl.tools.checkstyle.bdd</package>
<entry_point TYPE="method" FQNAME="com.puppycrawl.tools.checkstyle.bdd.InlineConfigParser void setProperties(com.puppycrawl.tools.checkstyle.bdd.ModuleInputConfiguration.Builder inputConfigBuilder, java.lang.String inputFilePath, java.util.List<java.lang.String> lines, int beginLineNo, java.lang.String moduleName, java.util.Map<java.lang.String,java.lang.String> defaultProperties)"/>
<problem_class id="ParametersPerMethod" severity="ERROR" attribute_key="ERRORS_ATTRIBUTES">Method with too many parameters</problem_class>
<description><code>setProperties()</code> has too many parameters (num parameters = 6) #loc</description>
<highlighted_element>setProperties</highlighted_element>
<language>JAVA</language>
<offset>24</offset>
<length>13</length>
</problem>

Pankraz76

nicely done now, try to increase cohesion.

rdiachenko

@Anmol202005 instead of having custom logic to load default properties, could you check if we can use XmlMetaReader.readAllModulesIncludingThirdPartyIfAny() please? From a quick glance, it does what you are trying to achieve and gives you a list of module objects including module properties.

Anmol202005 · 2025-04-22T09:35:54Z

@rdiachenko XmlMetaReader.readAllModulesIncludingThirdPartyIfAny() works well and really improves the readability.
Although there is a CI failure, can u please help out with it.

Pankraz76

sugar

rdiachenko · 2025-04-22T19:40:33Z

@rdiachenko XmlMetaReader.readAllModulesIncludingThirdPartyIfAny() works well and really improves the readability. Although there is a CI failure, can u please help out with it.

I don't see how it is related to your changes as mutations are in ImportControlCheck.java and other ImportXXX classes. Please try to update to latest master.

Pankraz76

try to avoid feature envy.

romani

item:

romani

item:

romani

item

romani

please do https://github.com/checkstyle/checkstyle/pull/16760/files#r2094188697

item

romani

items:

Pankraz76

thanks for consistent push, you will get this done.

Pankraz76

SOC

romani · 2025-05-18T13:01:31Z

@Anmol202005 , please fix unresovled items above.

…t properties

Anmol202005 · 2025-05-19T14:42:24Z

@Anmol202005 , please fix unresovled items above.

done.

romani

I am good with this .
I would like to make some call less nested (to avoid functional mess), but give it to next reviewee to decide

romani · 2025-05-19T16:00:12Z

I reset review status from Pankraz76, not be blocker for review.

Pankraz76

polish.

rdiachenko

lgtm

…nputs

Anmol202005 force-pushed the inlineConfig branch 3 times, most recently from 6921ccd to 258daf7 Compare April 5, 2025 07:20

Pankraz76 approved these changes Apr 5, 2025

View reviewed changes

Anmol202005 force-pushed the inlineConfig branch from 258daf7 to 6724da2 Compare April 6, 2025 07:53

rdiachenko reviewed Apr 6, 2025

View reviewed changes

Anmol202005 force-pushed the inlineConfig branch from 6724da2 to 2c89c40 Compare April 6, 2025 15:38

Anmol202005 mentioned this pull request Apr 9, 2025

Updating properties in Input files to mention all default properties, #16807

Open

Anmol202005 force-pushed the inlineConfig branch from 2c89c40 to 96b15b0 Compare April 9, 2025 15:49

rdiachenko requested changes Apr 9, 2025

View reviewed changes

Anmol202005 force-pushed the inlineConfig branch 2 times, most recently from b258725 to 951ddd5 Compare April 13, 2025 17:31

Pankraz76 approved these changes Apr 13, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Anmol202005 force-pushed the inlineConfig branch from 951ddd5 to d5c538f Compare April 14, 2025 17:22

rdiachenko reviewed Apr 14, 2025

View reviewed changes

Anmol202005 force-pushed the inlineConfig branch 3 times, most recently from 8c283d8 to 0bec7c6 Compare April 22, 2025 08:29

Pankraz76 reviewed Apr 22, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

rdiachenko reviewed Apr 22, 2025

View reviewed changes

Pankraz76 reviewed Apr 23, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Pankraz76 reviewed May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Pankraz76 suggested changes May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Anmol202005 force-pushed the inlineConfig branch from 7ef50dc to a26f673 Compare May 17, 2025 13:16

romani requested changes May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

romani requested changes May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Anmol202005 force-pushed the inlineConfig branch 3 times, most recently from 51242c1 to d8cd07f Compare May 17, 2025 18:46

romani requested changes May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

romani requested changes May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java Outdated

Pankraz76 reviewed May 17, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

Pankraz76 suggested changes May 18, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

Anmol202005 force-pushed the inlineConfig branch 3 times, most recently from e6d9571 to 998506a Compare May 19, 2025 12:52

Issue checkstyle#16378: Validate that all Inputs mentioned all defaul…

869bcf7

…t properties

Anmol202005 force-pushed the inlineConfig branch from 998506a to 869bcf7 Compare May 19, 2025 13:13

romani approved these changes May 19, 2025

View reviewed changes

romani assigned rdiachenko May 19, 2025

romani requested a review from Pankraz76 May 19, 2025 15:59

Pankraz76 approved these changes May 19, 2025

View reviewed changes

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

rdiachenko approved these changes May 19, 2025

View reviewed changes

rdiachenko assigned romani and unassigned rdiachenko May 19, 2025

romani merged commit 140f152 into checkstyle:master May 19, 2025
117 checks passed

harshittomar0201 added a commit to harshittomar0201/checkstyle that referenced this pull request Apr 2, 2026

Issue checkstyle#16760: update MultipleStringLiterals inline config i…

025e3ff

…nputs

Uh oh!

Conversation

Anmol202005 commented Apr 4, 2025

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Anmol202005 commented Apr 6, 2025

Uh oh!

rdiachenko left a comment

Choose a reason for hiding this comment

Uh oh!

Anmol202005 commented Apr 6, 2025

Uh oh!

rdiachenko commented Apr 6, 2025

Uh oh!

Anmol202005 commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 commented Apr 8, 2025

Uh oh!

Anmol202005 commented Apr 9, 2025

Uh oh!

rdiachenko commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdiachenko left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anmol202005 commented Apr 22, 2025

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdiachenko commented Apr 22, 2025

Uh oh!

Uh oh!

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

romani left a comment

Anmol202005 commented Apr 8, 2025 •

edited

Loading

rdiachenko commented Apr 9, 2025 •

edited

Loading

rdiachenko left a comment •

edited

Loading