Issue #17310: Added new Notes Macro by SteLeo1602 · Pull Request #17311 · checkstyle/checkstyle

SteLeo1602 · 2025-07-03T05:13:23Z

Zopsss · 2025-07-04T08:47:20Z

+    /**
+     * Assigns values to each instance variable.
+     *
+     * @param moduleName name of module.
+     * @return set of property names.
+     * @throws MacroExecutionException if the module could not be retrieved.
+     */
+    private static Set<String> getPropertyNames(String moduleName)
+            throws MacroExecutionException {
+        final Object instance = SiteUtil.getModuleInstance(moduleName);
+        final Class<?> clss = instance.getClass();
+
+        return SiteUtil.getPropertiesForDocumentation(clss, instance);
+    }


#17278 will be merged soon. DescriptionMacro.java also has this method. We should move it to a common place from where both of these files can access this method.

Should we move it to JavadocMetadataScraper class or create a new common Parent class which will have these methods @romani ?

moving to common should be delayed.
We will do unification with JavadocMetadataScraper, and reduction of doing the same, only after macroses fully cover our code base, and functionally we will be satisfied.
There will be a lot of surprises as move forward with migrations, so this implementation might change.

@SteLeo1602 , please create issue to move all potentially same methods to some common utility class to reuse between macroses.

@SteLeo1602 , please respond on above comment

Done: #17371

romani · 2025-07-06T13:47:12Z

@SteLeo1602 , please rebase.

✔ ~/java/github/romani/checkstyle [SteLeo1602/notesMacro L|✔] 
$ mvn clean test
...
[INFO] Running com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.048 s <<< FAILURE! -- in com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest
[ERROR] com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest.testAllCheckSectionJavaDocs -- Time elapsed: 2.046 s <<< ERROR!
java.lang.Error: Error was thrown while processing src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyCatchBlockCheck.java
	at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:320)
	at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:226)
	at com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest.assertCheckSection(XdocsJavaDocsTest.java:178)
	at com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest.testAllCheckSectionJavaDocs(XdocsJavaDocsTest.java:151)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: EmptyCatchBlock's class-level JavaDoc
diff (-expected +actual):
    @@ -1,4 +1,4 @@
    -<div>Checks for empty catch blocks. By default, check allows empty catch block with any comment inside.</div><div>Checks for empty catch blocks. By default, check allows empty catch block with any comment inside.</div> Notes:
    +<div>Checks for empty catch blocks. By default, check allows empty catch block with any comment inside.</div> Notes:
     <p>
     There are two options to make validation more precise: <b>exceptionVariableName</b> and <b>commentFormat</b>. If both options are specified - they are applied by <b>any of them is matching</b>.
     </p>
	at com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest$JavaDocCapture.visitClass(XdocsJavaDocsTest.java:638)
	at com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest$JavaDocCapture.visitToken(XdocsJavaDocsTest.java:586)
	at com.puppycrawl.tools.checkstyle.TreeWalker.notifyVisit(TreeWalker.java:393)
	at com.puppycrawl.tools.checkstyle.TreeWalker.processIter(TreeWalker.java:466)
	at com.puppycrawl.tools.checkstyle.TreeWalker.walk(TreeWalker.java:331)
	at com.puppycrawl.tools.checkstyle.TreeWalker.processFiltered(TreeWalker.java:215)
	at com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck.process(AbstractFileSetCheck.java:101)
	at com.puppycrawl.tools.checkstyle.Checker.processFile(Checker.java:340)
	at com.puppycrawl.tools.checkstyle.Checker.processFiles(Checker.java:299)
	at com.puppycrawl.tools.checkstyle.Checker.process(Checker.java:226)
	at com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest.assertCheckSection(XdocsJavaDocsTest.java:178)
	at com.puppycrawl.tools.checkstyle.internal.XdocsJavaDocsTest.testAllCheckSectionJavaDocs(XdocsJavaDocsTest.java:151)
	at [[Reflective call: 4 frames collapsed (https://goo.gl/aH3UyP)]].(:0)
	at [[Testing framework: 28 frames collapsed (https://goo.gl/aH3UyP)]].(:0)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at [[Testing framework: 9 frames collapsed (https://goo.gl/aH3UyP)]].(:0)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at [[Testing framework: 26 frames collapsed (https://goo.gl/aH3UyP)]].(:0)
	at org.apache.maven.surefire.junitplatform.LazyLauncher.execute(LazyLauncher.java:56)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:194)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:150)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:124)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

....
^C✘-INT ~/java/github/romani/checkstyle [SteLeo1602/notesMacro L|✚ 1] 
06:41 $ 
✘-INT ~/java/github/romani/checkstyle [SteLeo1602/notesMacro L|✚ 1] 
06:45 $ git diff
diff --git a/src/site/xdoc/checks/blocks/emptycatchblock.xml b/src/site/xdoc/checks/blocks/emptycatchblock.xml
index 8356df7eef..b00a65a7d8 100644
--- a/src/site/xdoc/checks/blocks/emptycatchblock.xml
+++ b/src/site/xdoc/checks/blocks/emptycatchblock.xml
@@ -16,9 +16,15 @@
       </subsection>
 
       <subsection name="Notes" id="Notes">
+        <div>
+          Checks for empty catch blocks.
+          By default, check allows empty catch block with any comment inside.
+        </div>
+
+Notes:
         <p>
           There are two options to make validation more precise: <b>exceptionVariableName</b> and
-          <b>commentFormat</b>.
+        <b>commentFormat</b>.
           If both options are specified - they are applied by <b>any of them is matching</b>.
         </p>
       </subsection>

macros is breathing , but polishing is required.

SteLeo1602 · 2025-07-09T04:23:45Z

GitHub, generate website

github-actions · 2025-07-09T04:31:02Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/a1d0bca_20250709043009/index.html

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/a1d0bca_20250709043009/checks/misc/todocomment.html#Notes

Vs
https://checkstyle.org/checks/misc/todocomment.html#TodoComment

It is good that notes comes closer to description

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/a1d0bca_20250709043009/checks/blocks/emptycatchblock.html#EmptyCatchBlock
Vs
https://checkstyle.org/checks/blocks/emptycatchblock.html#EmptyCatchBlock

No diff. Good.

romani

Items

romani

Items

Zopsss · 2025-07-13T03:17:54Z

+        if (NotesMacro.getNotesStartIndex(moduleJavadoc) > -1) {
+            descriptionEndIndex += NotesMacro.getNotesStartIndex(moduleJavadoc);
+        }


This is not a good way to use common methods. We shouldn't use method directly from other Macro. Instead we should have a common class for all Macros to use the util methods v

Done, the common util development is in progress in #17377

I can still see it's usage. For now just copy paste the common methods in macros. We'll move them toa common place as part of the issue you linked.

romani

Lets merge this, we can improve in followup PRs.
There two PRs already open that are based on this PR

Zopsss · 2025-07-14T02:35:00Z

+    /**
+     * Gets the start index of the Notes section.
+     *
+     * @param moduleJavadoc javadoc of module.
+     * @return start index.
+     */
+    public static int getNotesStartIndex(DetailNode moduleJavadoc) {


Why do we have this method inside description macro?

To check if there is Notes section in module's javadoc

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/site/DescriptionMacro.java

Lines 108 to 110 in 87656a3

if (NotesMacro.getNotesStartIndex(moduleJavadoc) > -1) {

descriptionEndIndex += NotesMacro.getNotesStartIndex(moduleJavadoc);

}

You have used NotesMacro's static method.

It will be the util's soon

Please follow this:#17311 (comment)

Do not use NotesMacro inside Description Macro. You have copy pasted the method in the Macro, just use it instead of using NotesMacro's public static method

Zopsss

LGTM

Zopsss reviewed Jul 4, 2025

View reviewed changes

SteLeo1602 force-pushed the notesMacro branch 2 times, most recently from fa37ce7 to a1d0bca Compare July 9, 2025 04:22

Zopsss suggested changes Jul 9, 2025

View reviewed changes

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java Outdated

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java Outdated

SteLeo1602 force-pushed the notesMacro branch from a1d0bca to 678225e Compare July 9, 2025 21:06

romani requested changes Jul 10, 2025

View reviewed changes

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java Outdated

romani requested changes Jul 10, 2025

View reviewed changes

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java Outdated

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java Outdated

SteLeo1602 force-pushed the notesMacro branch 2 times, most recently from 7354a09 to 3db3e20 Compare July 11, 2025 06:56

Zopsss suggested changes Jul 11, 2025

View reviewed changes

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java Outdated

SteLeo1602 force-pushed the notesMacro branch from 3db3e20 to e449f65 Compare July 11, 2025 22:37

SteLeo1602 mentioned this pull request Jul 12, 2025

Move the same class members from macroses to new common utility class #17371

Closed

Zopsss reviewed Jul 12, 2025

View reviewed changes

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/NotesMacro.java

SteLeo1602 force-pushed the notesMacro branch from e449f65 to 7d22053 Compare July 13, 2025 00:28

Zopsss reviewed Jul 13, 2025

View reviewed changes

SteLeo1602 force-pushed the notesMacro branch from 7d22053 to 87656a3 Compare July 13, 2025 04:01

romani approved these changes Jul 13, 2025

View reviewed changes

Zopsss reviewed Jul 14, 2025

View reviewed changes

Issue checkstyle#17310: Added new Notes Macro

62e8c9e

SteLeo1602 force-pushed the notesMacro branch from 87656a3 to 62e8c9e Compare July 14, 2025 03:11

Zopsss approved these changes Jul 14, 2025

View reviewed changes

romani merged commit 6f07680 into checkstyle:master Jul 14, 2025
117 checks passed

	if (NotesMacro.getNotesStartIndex(moduleJavadoc) > -1) {
	descriptionEndIndex += NotesMacro.getNotesStartIndex(moduleJavadoc);
	}

Uh oh!

Conversation

SteLeo1602 commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteLeo1602 Jul 12, 2025 • 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!

romani commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SteLeo1602 commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 9, 2025 • edited by romani Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zopsss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SteLeo1602 commented Jul 3, 2025 •

edited

Loading

SteLeo1602 Jul 12, 2025 •

edited

Loading

romani commented Jul 6, 2025 •

edited

Loading

github-actions Bot commented Jul 9, 2025 •

edited by romani

Loading