Merge pull request #1591 from theKBro/fixCmakeWarningsMultilineAndWrongDirectyIssues

Preserve CMake diagnostic messages and avoid incorrect build-directory prefixes

Author: uhafner

src/main/java/edu/hm/hafner/analysis/LookaheadParser.java

@@ -70,7 +70,9 @@ private void parse(final Report report, final LookaheadStream lookahead) {
         try (var builder = new IssueBuilder()) {
             while (lookahead.hasNext()) {
                 var line = lookahead.next();
-                handleDirectoryChanges(builder, line, report);
+                if (isDirectoryTrackingEnabled()) {
+                    handleDirectoryChanges(builder, line, report);
+                }
                 preprocessLine(line);
                 if (isLineInteresting(line)) {
                     var matcher = pattern.matcher(line);
@@ -95,6 +97,19 @@ protected void preprocessLine(final String line) {
         // empty default implementation does nothing
     }
 
+    /**
+     * Returns whether build-tool directory messages should set the base directory for relative issue filenames.
+     * Tracking is enabled by default for compiler diagnostics, including Ninja builds that use CMake's binary directory
+     * as their working directory. The CMake marker is a directory hint, not proof of a compiler's working directory.
+     * Parsers whose filenames are not relative to the build working directory may opt out without losing lookahead or
+     * multiline parsing.
+     *
+     * @return {@code true} to track Make directory changes and CMake binary-directory markers
+     */
+    protected boolean isDirectoryTrackingEnabled() {
+        return true;
+    }
+
     /**
      * When changing directories using 'Entering directory' output, save new directory to our stack for later use, then
      * return it for use now.

src/main/java/edu/hm/hafner/analysis/parser/CMakeParser.java

@@ -6,12 +6,16 @@
 import edu.hm.hafner.analysis.Severity;
 import edu.hm.hafner.util.LookaheadStream;
 import java.io.Serial;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Optional;
 import java.util.regex.Matcher;
 import org.apache.commons.lang3.StringUtils;
 
 /**
- * A parser for CMake warnings.
+ * A parser for CMake warnings. Keeps the source filenames reported by CMake: build-tool working directories and CMake's
+ * binary-directory marker do not identify the source directory of a configure diagnostic. Relative filenames remain
+ * relative for consumers to resolve using source-tree context.
  *
  * @author Uwe Brandt
  */
@@ -27,24 +31,51 @@ public CMakeParser() {
         super(CMAKE_WARNING_PATTERN);
     }
 
+    @Override
+    protected boolean isDirectoryTrackingEnabled() {
+        return false;
+    }
+
     @Override
     protected Optional<Issue> createIssue(
             final Matcher matcher, final LookaheadStream lookahead, final IssueBuilder builder) {
         // if the category is contained in brackets, remove those brackets
         var category = StringUtils.strip(matcher.group("category"), "()");
-        int prefixLength = matcher.group("prefix").length();
+        var prefix = matcher.group("prefix");
         return builder.setFileName(matcher.group("file"))
                 .setLineStart(matcher.group("line"))
                 .setCategory(category)
-                .setMessage(readMessage(lookahead, prefixLength))
+                .setMessage(readMessage(lookahead, prefix))
                 .setSeverity(Severity.guessFromString(matcher.group("type")))
                 .buildOptional();
     }
 
-    private String readMessage(final LookaheadStream lookahead, final int prefixLength) {
-        if (lookahead.hasNext()) {
-            return StringUtils.substring(lookahead.next(), prefixLength).trim();
+    private String readMessage(final LookaheadStream lookahead, final String prefix) {
+        List<String> messageLines = new ArrayList<>();
+        while (lookahead.hasNext()) {
+            var line = removePrefix(lookahead.peekNext(), prefix);
+            if (!isContinuation(line)) {
+                break;
+            }
+            lookahead.next();
+            messageLines.add(line.strip());
+        }
+
+        while (!messageLines.isEmpty()
+                && messageLines.get(messageLines.size() - 1).isEmpty()) {
+            messageLines.remove(messageLines.size() - 1);
+        }
+        return String.join("\n", messageLines);
+    }
+
+    private String removePrefix(final String line, final String prefix) {
+        if (line.startsWith(prefix)) {
+            return line.substring(prefix.length());
         }
-        return "";
+        return line.equals(prefix.stripTrailing()) ? "" : line;
+    }
+
+    private boolean isContinuation(final String line) {
+        return line.isEmpty() || Character.isWhitespace(line.charAt(0));
     }
 }

src/test/java/edu/hm/hafner/analysis/parser/CMakeParserTest.java

@@ -4,6 +4,9 @@
 import edu.hm.hafner.analysis.Severity;
 import edu.hm.hafner.analysis.assertions.SoftAssertions;
 import edu.hm.hafner.analysis.registry.AbstractParserTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * Tests the class {@link CMakeParser}.
@@ -68,6 +71,135 @@ protected void assertThatIssuesArePresent(final Report report, final SoftAsserti
                 .hasFileName("unlikely.cmake");
     }
 
+    @ParameterizedTest
+    @ValueSource(
+            strings = {
+                "-- Build files have been written to: C:/workspace/build",
+                "22>-- Build files have been written to: C:/workspace/build",
+                "[timestamp] C/C++: -- Build files have been written to: C:/workspace/build",
+                "make[1]: Entering directory 'C:/workspace/build'",
+                "make[1]: Leaving directory 'C:/workspace/build'",
+                "-- Build files have an unrelated message",
+                "Entering directory without a valid path"
+            })
+    void shouldIgnoreBuildDirectoryMessages(final String directoryMessage) {
+        var report = parseStringContent(directoryMessage + """
+
+                CMake Warning at subproject/CMakeLists.txt:14 (message):
+                  Configure warning
+                """);
+
+        try (var softly = new SoftAssertions()) {
+            softly.assertThat(report).hasSize(1).doesNotHaveErrors();
+            softly.assertThat(report.get(0))
+                    .hasFileName("subproject/CMakeLists.txt")
+                    .hasPath("-")
+                    .hasLineStart(14)
+                    .hasMessage("Configure warning");
+        }
+    }
+
+    @Test
+    void shouldPreserveAbsoluteSourcePaths() {
+        var report = parseStringContent("""
+                -- Build files have been written to: C:/workspace/build
+                CMake Warning at C:/workspace/source/cmake/options.cmake:7 (message):
+                  Warning with an absolute source path
+                """);
+
+        try (var softly = new SoftAssertions()) {
+            softly.assertThat(report).hasSize(1).doesNotHaveErrors();
+            softly.assertThat(report.get(0))
+                    .hasFileName("C:/workspace/source/cmake/options.cmake")
+                    .hasLineStart(7)
+                    .hasMessage("Warning with an absolute source path");
+        }
+    }
+
+    @Test
+    void shouldNotCarryDirectoriesBetweenProjects() {
+        var report = parseStringContent("""
+                make: Entering directory 'C:/workspace/build'
+                -- Build files have been written to: C:/workspace/build/external
+                CMake Warning at external/CMakeLists.txt:3 (message):
+                  External warning
+                make: Leaving directory 'C:/workspace/build'
+                -- Build files have been written to: C:/workspace/build/root
+                CMake Warning at CMakeLists.txt:9 (message):
+                  Root warning
+                """);
+
+        try (var softly = new SoftAssertions()) {
+            softly.assertThat(report).hasSize(2).doesNotHaveErrors();
+            softly.assertThat(report.get(0))
+                    .hasFileName("external/CMakeLists.txt")
+                    .hasMessage("External warning");
+            softly.assertThat(report.get(1)).hasFileName("CMakeLists.txt").hasMessage("Root warning");
+        }
+    }
+
+    @Test
+    void shouldReadPrefixedMultilineMessages() {
+        var report = parseStringContent("""
+                [step] CMake Warning at CMakeLists.txt:9 (message):
+                [step]   First line
+                [step]
+                [step]   A wrapped explanation, continued
+                [step]   on another line.
+                [step]
+                [step] CMake Warning at nested/CMakeLists.txt:4 (message):
+                [step]   Next warning
+                """);
+
+        try (var softly = new SoftAssertions()) {
+            softly.assertThat(report).hasSize(2).doesNotHaveErrors();
+            softly.assertThat(report.get(0)).hasFileName("CMakeLists.txt").hasMessage("""
+                        First line
+
+                        A wrapped explanation, continued
+                        on another line.""");
+            softly.assertThat(report.get(1))
+                    .hasFileName("nested/CMakeLists.txt")
+                    .hasMessage("Next warning");
+        }
+    }
+
+    @Test
+    void shouldExcludeCallStackFromMessage() {
+        var report = parseStringContent("""
+                CMake Warning at CMakeLists.txt:14 (message):
+                  Warning raised by the nested project
+                Call Stack (most recent call first):
+                  external-project/CMakeLists.txt:4 (include)
+                  CMakeLists.txt:38 (ExternalProject_Add)
+                CMake Warning at after.cmake:3 (message):
+                  Warning after the call stack
+                """);
+
+        try (var softly = new SoftAssertions()) {
+            softly.assertThat(report).hasSize(2).doesNotHaveErrors();
+            softly.assertThat(report.get(0))
+                    .hasFileName("CMakeLists.txt")
+                    .hasMessage("Warning raised by the nested project");
+            softly.assertThat(report.get(1)).hasFileName("after.cmake").hasMessage("Warning after the call stack");
+        }
+    }
+
+    @Test
+    void shouldNotConsumeWarningAfterEmptyMessage() {
+        var report = parseStringContent("""
+                CMake Warning at empty.cmake:3 (message):
+                CMake Warning at after.cmake:4 (message):
+                  Next warning
+                """);
+
+        try (var softly = new SoftAssertions()) {
+            softly.assertThat(report).hasSize(2).doesNotHaveErrors();
+            softly.assertThat(report.get(0)).hasFileName("empty.cmake").hasMessage("");
+            softly.assertThat(report.get(1)).hasFileName("after.cmake").hasMessage("Next warning");
+        }
+    }
+
     @Override
     protected CMakeParser createParser() {
         return new CMakeParser();