From 3bfc86bde7ef708104be72f8357cf678bf001647 Mon Sep 17 00:00:00 2001 From: Tom Tourwe Date: Tue, 13 Oct 2026 10:31:43 +0200 Subject: [PATCH] An enum constant carries its comments, and a javadoc says where it is written (TestEnumConstantComments) The comment pre-scan (SourceCodeScan.scanTypeBody) had no case for an enum constant, so a // or /* */ comment above a constant, or a // comment after it on its line ("B, // the second"), was attached to nothing: FieldInfo.comments() of every enum constant held at most its javadoc. A tool removing a dead constant could not tell which comment went with it. Enum constants are now scanned like field declarations; the lookahead for the comment after the constant looks past the ',' or ';' that follows it, since that comment precedes the next constant's first token and would otherwise become the next constant's leading comment. A javadoc's source spans its TEXT, not the comment: "/** the second */ B," gives the javadoc column 9 (the 't'), not column 5 (the '/'). Consumers count from there to place a word inside the comment (rename in comments), so that stays. The comment as written, "/**" through "*/", is now on that source as the new detail DetailedSources.WHOLE_COMMENT, for a consumer that removes or moves the comment itself and cannot infer its first character when the line also holds code. The unit's text is read once per compilation unit, on the first javadoc that needs it. maddi build: 2586 tests, 0 failures; maddi-mod build: 1668 tests, 0 failures (both against this branch). Signed-off-by: Tom Tourwe --- .../cst/api/element/DetailedSources.java | 8 ++ .../java/openjdk/ScanCompilationUnit.java | 3 +- .../maddi/java/openjdk/ScanJavaDoc.java | 58 ++++++++++- .../maddi/java/openjdk/SourceCodeScan.java | 58 ++++++++--- .../other/TestEnumConstantComments.java | 97 +++++++++++++++++++ 5 files changed, 210 insertions(+), 14 deletions(-) create mode 100644 maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/other/TestEnumConstantComments.java diff --git a/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/element/DetailedSources.java b/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/element/DetailedSources.java index 1179d22fe..d197c96f3 100644 --- a/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/element/DetailedSources.java +++ b/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/element/DetailedSources.java @@ -73,6 +73,14 @@ public interface DetailedSources { * Source range spanning the entire field declaration (type + name + initialiser). */ Object FIELD_DECLARATION = new Object(); + /** + * Source range of a javadoc comment as written, from its opening {@code /**} through its closing + * {@code */}, set on the {@link JavaDoc}'s own source. That source spans the comment's TEXT only, and + * consumers placing a word inside the comment count from it. This range is for a consumer that removes or moves + * the comment itself and needs its first character -- which it cannot infer when the line also holds code, as + * in {@code /** the second */ B,}. + */ + Object WHOLE_COMMENT = new Object(); /** * Position of the comma that precedes an element in a comma-separated list. * Computed for formal type parameter lists, formal method parameter lists, field declaration lists, and diff --git a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java index 9ffb30dd9..bb91beb92 100644 --- a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java +++ b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java @@ -117,7 +117,8 @@ private record BlockData(Block.Builder blockBuilder, String index, int numberOfS classSymbolScanner.syntheticListField()); DocSourcePositions docSourcePositions = docTrees.getSourcePositions(); - this.scanJavaDoc = new ScanJavaDoc(runtime, typeData, docSourcePositions, compilationUnitTree, lineMap); + this.scanJavaDoc = new ScanJavaDoc(runtime, typeData, docSourcePositions, compilationUnitTree, lineMap, + new ScanJavaDoc.SourceText(compilationUnitTree)); convertType = classSymbolScanner; convertType.startCompilationUnit(this, elementStack); diff --git a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanJavaDoc.java b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanJavaDoc.java index e8ccbaa9d..31c3efc98 100644 --- a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanJavaDoc.java +++ b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanJavaDoc.java @@ -7,11 +7,14 @@ import com.sun.source.util.DocTreeScanner; import com.sun.tools.javac.tree.DCTree; import com.sun.tools.javac.util.JCDiagnostic; +import io.codelaser.maddi.cst.api.element.DetailedSources; import io.codelaser.maddi.cst.api.element.JavaDoc; import io.codelaser.maddi.cst.api.element.Source; import io.codelaser.maddi.cst.api.runtime.Runtime; +import org.jetbrains.annotations.Nullable; import javax.tools.Diagnostic; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -20,16 +23,69 @@ public record ScanJavaDoc(Runtime runtime, TypeData typeData, DocSourcePositions docSourcePositions, CompilationUnitTree compilationUnitTree, - LineMap lineMap) { + LineMap lineMap, + SourceText sourceText) { public JavaDoc scan(DocCommentTree docCommentTree) { MyScanner myScanner = new MyScanner(docCommentTree); Source source = myScanner.source(docCommentTree); if (source == runtime.noSource()) source = rawCommentSource(docCommentTree); + Source wholeComment = wholeCommentSource(docCommentTree); + if (wholeComment != null && source != runtime.noSource()) { + source = source.withDetailedSources(runtime.newDetailedSourcesBuilder() + .put(DetailedSources.WHOLE_COMMENT, wholeComment).build()); + } myScanner.scan(docCommentTree, null); return runtime.newJavaDoc(source, myScanner.comment.toString(), List.copyOf(myScanner.tags)); } + /** + * The text of the compilation unit, read once and only when a javadoc asks for it: {@link #scan} runs for every + * javadoc of the unit, and the file object may go back to the disk on each read. + */ + static final class SourceText { + private final CompilationUnitTree compilationUnitTree; + private String text; + private boolean read; + + SourceText(CompilationUnitTree compilationUnitTree) { + this.compilationUnitTree = compilationUnitTree; + } + + @Nullable String get() { + if (!read) { + read = true; + try { + text = compilationUnitTree.getSourceFile().getCharContent(true).toString(); + } catch (IOException | RuntimeException e) { + text = null; + } + } + return text; + } + } + + /** + * The comment as written, {@code /**} through {@code */} (see {@link DetailedSources#WHOLE_COMMENT}). The + * tokenizer's comment knows where its {@code /**} is, as in {@link #rawCommentSource}; the closing + * {@code */} is the first one after it, since a comment cannot contain one. Null when either end cannot be + * found. + */ + private @Nullable Source wholeCommentSource(DocCommentTree docCommentTree) { + if (!(docCommentTree instanceof DCTree.DCDocComment dc) || dc.comment == null) return null; + JCDiagnostic.DiagnosticPosition pos = dc.comment.getPos(); + if (pos == null) return null; + int start = pos.getStartPosition(); + if (start == Diagnostic.NOPOS) return null; + String text = sourceText.get(); + if (text == null || !text.startsWith("/**", start)) return null; + int close = text.indexOf("*/", start + 3); + if (close < 0) return null; + int lastChar = close + 1; // inclusive: the '/' of '*/' + return runtime.newParserSource("-", (int) lineMap.getLineNumber(start), (int) lineMap.getColumnNumber(start), + (int) lineMap.getLineNumber(lastChar), (int) lineMap.getColumnNumber(lastChar)); + } + /** * #31: javac positions a doc comment tree by its BODY, so an empty {@code /** */} has no position at all, * and the JavaDoc read as line 0 to every consumer placing something relative to a member's comments. The diff --git a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/SourceCodeScan.java b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/SourceCodeScan.java index 75861dd39..cd0e816b8 100644 --- a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/SourceCodeScan.java +++ b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/SourceCodeScan.java @@ -370,12 +370,41 @@ private void scanTypeBody(Node body) { case CompactConstructorDeclaration ccd -> scanMethodDeclaration(ccd); case MethodDeclaration _, AnnotationMethodDeclaration _ -> scanMethodDeclaration(node2); case FieldDeclaration fd -> scanFieldDeclaration(fd); + case EnumConstant ec -> scanEnumConstant(ec); default -> { } } } } + /** + * ⛔ AN ENUM CONSTANT HAD NO COMMENTS AT ALL. This switch had no case for it, so a {@code //} or {@code /* */} + * comment in front of a constant was recorded for nothing, and a consumer removing the constant could not tell + * that the comment belonged to it: removing B from + * 
+     *     // the first
+     *     A,
+     *     // the second
+     *     B,
+     *     // the third
+     *     C;
+     *
+ * deleted from B's name to C's name, which took "// the third" and left "// the second" above C. + *

+ * A single-line comment after the constant on its last line -- {@code B, // the second} -- belongs to B as + * well. The comma (or the ';' that ends the list) stands between B and that comment, so the lookahead looks + * past it: the comment is a preceding token of whatever comes after the separator, and without this the next + * constant takes it as its own leading comment. + */ + private void scanEnumConstant(EnumConstant ec) { + Node after = ec.nextSibling(); + if (after != null && (after.getType() == Token.TokenType.COMMA + || after.getType() == Token.TokenType.SEMICOLON)) { + after = after.nextSibling(); + } + addComments(ec, after); + } + private void scanModifiers(Modifiers modifiers) { for (Node node : modifiers.children()) { String string = node.getSource(); @@ -689,19 +718,24 @@ private void visit(Node node, Predicate test) { */ private void addComments(Node node, boolean lookahead) { + addComments(node, lookahead ? node.nextSibling() : null); + } + + /** + * @param nextSibling the node whose preceding comments are searched for a single-line comment on the last line + * of {@code node}; null for no lookahead + */ + private void addComments(Node node, @Nullable Node nextSibling) { Source source = source(node); - if (lookahead) { - Node nextSibling = node.nextSibling(); - if (nextSibling != null) { - List comments = comments(nextSibling) - .filter(c -> c.source().endLine() == c.source().beginLine() - && c.source().beginLine() == source.endLine()) - .filter(c -> seen.add(c.source())) - .toList(); - if (!comments.isEmpty()) { - List prev = this.comments.put(source, comments); - assert prev == null; - } + if (nextSibling != null) { + List comments = comments(nextSibling) + .filter(c -> c.source().endLine() == c.source().beginLine() + && c.source().beginLine() == source.endLine()) + .filter(c -> seen.add(c.source())) + .toList(); + if (!comments.isEmpty()) { + List prev = this.comments.put(source, comments); + assert prev == null; } } List comments = comments(node).filter(c -> seen.add(c.source())).toList(); diff --git a/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/other/TestEnumConstantComments.java b/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/other/TestEnumConstantComments.java new file mode 100644 index 000000000..4fbb1addf --- /dev/null +++ b/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/other/TestEnumConstantComments.java @@ -0,0 +1,97 @@ +/* + * maddi: a modification analyzer for duplication detection and immutability. + * Copyright 2020-2025, Bart Naudts, https://github.com/CodeLaser/maddi + * + * This program is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free Software + * Foundation, either version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for + * more details. You should have received a copy of the GNU Lesser General Public + * License along with this program. If not, see . + */ + +package io.codelaser.maddi.java.openjdk.other; + +import io.codelaser.maddi.cst.api.element.Comment; +import io.codelaser.maddi.cst.api.element.DetailedSources; +import io.codelaser.maddi.cst.api.element.Source; +import io.codelaser.maddi.cst.api.info.TypeInfo; +import io.codelaser.maddi.java.openjdk.CommonTest; +import org.intellij.lang.annotations.Language; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * An enum constant carries the comments written for it, as a field declaration does: a comment above it, and a + * single-line comment after it on its own line. Until this test the comment pre-scan had no case for an enum + * constant, so both were recorded for nothing, and a consumer removing a constant could not tell which comment went + * with it. + *

+ * And a javadoc carries where it is WRITTEN, {@code /**} through {@code */}, as + * {@link DetailedSources.WHOLE_COMMENT} on its source. The javadoc's own source spans its text only, so on a line + * shared with code -- {@code /** the second */ B,} -- nothing said where the comment began. + */ +public class TestEnumConstantComments extends CommonTest { + + @Language("java") + private static final String INPUT = """ + package a.b; + public enum E { + // above A + A, // after A + /* above B */ + B, + /** before C */ C, + /** + * Above D. + */ + D; // after D + } + """; + + @DisplayName("each enum constant has the comments written for it, and a javadoc knows where it is written") + @Test + public void test() { + TypeInfo e = scan("a.b.E", INPUT); + + assertEquals("[SingleLineComment at 3:5, SingleLineComment at 4:8]", comments(e, "A"), + "A: '// above A' on the line above it (line 3, column 5), then '// after A' after it on its own" + + " line (line 4, column 8)"); + assertEquals("[MultiLineComment at 5:5]", comments(e, "B"), + "B: '/* above B */' on the line above it (line 5, column 5), and not A's '// after A'"); + assertEquals("[JavaDoc at 7:9]", comments(e, "C"), + "C: its javadoc, whose source starts at its text 'before C' (line 7, column 9)"); + assertEquals("[JavaDoc at 9:8, SingleLineComment at 11:8]", comments(e, "D"), + "D: its javadoc block, whose source starts at its text 'Above D.' (line 9, column 8), then" + + " '// after D' after the ';' on its line (line 11, column 8)"); + + assertEquals("7:5-7:19", wholeComment(e, "C"), + "C's javadoc as written: '/** before C */' runs from column 5 to column 19 of line 7"); + assertEquals("8:5-10:7", wholeComment(e, "D"), + "D's javadoc as written: '/**' at line 8 column 5 through '*/' at line 10 columns 6-7"); + } + + /** Each comment of the constant as its kind and the line:column where its source begins. */ + private static String comments(TypeInfo e, String constant) { + List comments = e.getFieldByName(constant, true).comments(); + return comments.stream() + .map(c -> c.getClass().getSimpleName().replace("Impl", "") + + " at " + c.source().beginLine() + ":" + c.source().beginPos()) + .toList().toString(); + } + + private static String wholeComment(TypeInfo e, String constant) { + Source source = e.getFieldByName(constant, true).javaDoc().source(); + return position(source.detailedSources().detail(DetailedSources.WHOLE_COMMENT)); + } + + private static String position(Source s) { + return s.beginLine() + ":" + s.beginPos() + "-" + s.endLine() + ":" + s.endPos(); + } +}