From 69da519ab8e86f84efbb923722b9a26754434fe5 Mon Sep 17 00:00:00 2001 From: Jonah Jeleniewski Date: Mon, 31 Mar 2025 11:50:55 +1100 Subject: [PATCH] Implement `ParsingError` check --- CHANGELOG.md | 1 + .../integradev/delphi/checks/CheckList.java | 1 + .../delphi/checks/ParsingErrorCheck.java | 34 +++++++++ .../rules/community-delphi/ParsingError.html | 6 ++ .../rules/community-delphi/ParsingError.json | 19 +++++ .../delphi/checks/CheckTestNameTest.java | 2 + .../au/com/integradev/delphi/antlr/Delphi.g | 28 ++++++-- .../integradev/delphi/file/DelphiFile.java | 2 +- .../com/integradev/delphi/DelphiSensor.java | 40 ++++++++++- .../integradev/delphi/DelphiSensorTest.java | 69 +++++++++++++++++-- 10 files changed, 190 insertions(+), 12 deletions(-) create mode 100644 delphi-checks/src/main/java/au/com/integradev/delphi/checks/ParsingErrorCheck.java create mode 100644 delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.html create mode 100644 delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e3579ecc..55866f156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `ParsingError` analysis rule, which flags files where parsing failures occurred. - Support for the `WEAK_NATIVEINT` symbol, which is defined from Delphi 12 onward. - Support for undocumented intrinsics usable within `varargs` routines: - `VarArgStart` diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java index a619ae562..fc99023ee 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java @@ -115,6 +115,7 @@ public final class CheckList { NoSonarCheck.class, NonLinearCastCheck.class, ObjectTypeCheck.class, + ParsingErrorCheck.class, PascalStyleResultCheck.class, PlatformDependentCastCheck.class, PlatformDependentTruncationCheck.class, diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/ParsingErrorCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/ParsingErrorCheck.java new file mode 100644 index 000000000..325aa79a7 --- /dev/null +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/ParsingErrorCheck.java @@ -0,0 +1,34 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2025 Integrated Application Development + * + * 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, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.checks; + +import org.sonar.check.Rule; +import org.sonar.plugins.communitydelphi.api.ast.DelphiAst; +import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; +import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; + +@Rule(key = "ParsingError") +public class ParsingErrorCheck extends DelphiCheck { + @Override + public DelphiCheckContext visit(DelphiAst ast, DelphiCheckContext context) { + // Dummy implementation to satisfy the check registrar. + // See DelphiSensor::handleParsingError. + return context; + } +} diff --git a/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.html b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.html new file mode 100644 index 000000000..19edce869 --- /dev/null +++ b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.html @@ -0,0 +1,6 @@ +

Why is this an issue?

+

+ When the Delphi parser fails, it is possible to record the failure as a violation on the file. + This way, not only it is possible to track the number of files that do not parse but also to + easily find out why they do not parse. +

diff --git a/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.json b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.json new file mode 100644 index 000000000..c3ed75cff --- /dev/null +++ b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/ParsingError.json @@ -0,0 +1,19 @@ +{ + "title": "Delphi parser failure", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant/Issue", + "constantCost": "30min" + }, + "code": { + "attribute": "CONVENTIONAL", + "impacts": { + "MAINTAINABILITY": "MEDIUM" + } + }, + "tags": ["suspicious"], + "defaultSeverity": "Major", + "scope": "ALL", + "quickfix": "unknown" +} diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/CheckTestNameTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/CheckTestNameTest.java index 5d959c613..94f695a37 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/CheckTestNameTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/CheckTestNameTest.java @@ -208,6 +208,8 @@ void testChecksHaveAnAssociatedTest() { .areAssignableTo(DelphiCheck.class) .and() .doNotHaveModifier(JavaModifier.ABSTRACT) + .and() + .doNotBelongToAnyOf(ParsingErrorCheck.class) .should(HAVE_ASSOCIATED_TEST) .allowEmptyShould(true) .check(CHECKS_PACKAGE); diff --git a/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g b/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g index 8cff11310..3ccc1cde5 100644 --- a/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g +++ b/delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g @@ -143,7 +143,7 @@ import org.apache.commons.lang3.StringUtils; public void reportError(RecognitionException e) { String hdr = this.getErrorHeader(e); String msg = this.getErrorMessage(e, this.getTokenNames()); - throw new LexerException(hdr + " " + msg, e); + throw new LexerException(hdr + " " + msg, e.line, e); } @Override @@ -152,12 +152,20 @@ import org.apache.commons.lang3.StringUtils; } public static class LexerException extends RuntimeException { - public LexerException(String message) { + private final int line; + + public LexerException(String message, int line) { super(message); + this.line = line; } - public LexerException(String message, Throwable cause) { + public LexerException(String message, int line, Throwable cause) { super(message, cause); + this.line = line; + } + + public int getLine() { + return line; } } @@ -234,7 +242,8 @@ import org.apache.commons.lang3.StringUtils; + state.tokenStartLine + ":" + state.tokenStartCharPositionInLine - + " unterminated multi-line comment"); + + " unterminated multi-line comment", + state.tokenStartLine); default: // do nothing @@ -350,7 +359,7 @@ import org.apache.commons.lang3.StringUtils; public void reportError(RecognitionException e) { String hdr = this.getErrorHeader(e); String msg = this.getErrorMessage(e, this.getTokenNames()); - throw new ParserException(hdr + " " + msg, e); + throw new ParserException(hdr + " " + msg, e.line, e); } @Override @@ -373,8 +382,15 @@ import org.apache.commons.lang3.StringUtils; } public static class ParserException extends RuntimeException { - public ParserException(String message, Throwable cause) { + private final int line; + + public ParserException(String message, int line, Throwable cause) { super(message, cause); + this.line = line; + } + + public int getLine() { + return line; } } } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/file/DelphiFile.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/file/DelphiFile.java index 2f57d3c1e..27a569312 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/file/DelphiFile.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/file/DelphiFile.java @@ -183,7 +183,7 @@ private static DelphiAst createAST( token.getChannel() == Token.HIDDEN_CHANNEL || token.getType() == Token.EOF); if (isEmptyFile) { - throw new EmptyDelphiFileException("Empty files are not allowed."); + throw new EmptyDelphiFileException("Empty files are not allowed"); } DelphiParser parser = new DelphiParser(tokenStream); diff --git a/sonar-delphi-plugin/src/main/java/au/com/integradev/delphi/DelphiSensor.java b/sonar-delphi-plugin/src/main/java/au/com/integradev/delphi/DelphiSensor.java index 38910bb12..e2bbf7df5 100644 --- a/sonar-delphi-plugin/src/main/java/au/com/integradev/delphi/DelphiSensor.java +++ b/sonar-delphi-plugin/src/main/java/au/com/integradev/delphi/DelphiSensor.java @@ -25,6 +25,8 @@ import static au.com.integradev.delphi.utils.DelphiUtils.inputFilesToPaths; import static au.com.integradev.delphi.utils.DelphiUtils.stopProgressReport; +import au.com.integradev.delphi.antlr.DelphiLexer.LexerException; +import au.com.integradev.delphi.antlr.DelphiParser.ParserException; import au.com.integradev.delphi.compiler.CompilerVersion; import au.com.integradev.delphi.compiler.Toolchain; import au.com.integradev.delphi.core.Delphi; @@ -33,6 +35,7 @@ import au.com.integradev.delphi.file.DelphiFile; import au.com.integradev.delphi.file.DelphiFile.DelphiFileConstructionException; import au.com.integradev.delphi.file.DelphiFile.DelphiInputFile; +import au.com.integradev.delphi.file.DelphiFile.EmptyDelphiFileException; import au.com.integradev.delphi.file.DelphiFileConfig; import au.com.integradev.delphi.msbuild.DelphiProjectHelper; import au.com.integradev.delphi.preprocessor.DelphiPreprocessorFactory; @@ -51,6 +54,9 @@ import org.sonar.api.batch.sensor.Sensor; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.SensorDescriptor; +import org.sonar.api.batch.sensor.issue.NewIssue; +import org.sonar.api.batch.sensor.issue.NewIssueLocation; +import org.sonar.api.rule.RuleKey; import org.sonarsource.analyzer.commons.ProgressReport; public class DelphiSensor implements Sensor { @@ -135,13 +141,14 @@ private void executeOnFiles(SensorContext sensorContext) { try { for (Path sourceFile : sourceFiles) { String absolutePath = sourceFile.toAbsolutePath().toString(); + InputFile inputFile = delphiProjectHelper.getFile(absolutePath); try { - InputFile inputFile = delphiProjectHelper.getFile(absolutePath); DelphiInputFile delphiFile = DelphiInputFile.from(inputFile, config); executor.execute(executorContext, delphiFile); progressReport.nextFile(); } catch (DelphiFileConstructionException e) { LOG.error("Error while analyzing {}", absolutePath, e); + handleParsingError(sensorContext, inputFile, e); } } success = true; @@ -150,6 +157,37 @@ private void executeOnFiles(SensorContext sensorContext) { } } + private static void handleParsingError( + SensorContext context, InputFile inputFile, DelphiFileConstructionException e) { + Throwable cause = e.getCause(); + if (cause instanceof LexerException + || cause instanceof ParserException + || cause instanceof EmptyDelphiFileException) { + NewIssue newIssue = + context.newIssue().forRule(RuleKey.of("community-delphi", "ParsingError")); + + NewIssueLocation primaryLocation = + newIssue + .newLocation() + .on(inputFile) + .message(String.format("Parse error (%s)", cause.getMessage())); + + int line = 0; + if (cause instanceof ParserException) { + line = ((ParserException) cause).getLine(); + } else if (cause instanceof LexerException) { + line = ((LexerException) cause).getLine(); + } + + if (line != 0) { + primaryLocation.at(inputFile.selectLine(line)); + } + + newIssue.at(primaryLocation); + newIssue.save(); + } + } + private SearchPath createSearchPath() { /* CodeGear.Delphi.Targets appends the library paths to DCC_UnitSearchPath to create a new diff --git a/sonar-delphi-plugin/src/test/java/au/com/integradev/delphi/DelphiSensorTest.java b/sonar-delphi-plugin/src/test/java/au/com/integradev/delphi/DelphiSensorTest.java index acbc66d39..7618e892a 100644 --- a/sonar-delphi-plugin/src/test/java/au/com/integradev/delphi/DelphiSensorTest.java +++ b/sonar-delphi-plugin/src/test/java/au/com/integradev/delphi/DelphiSensorTest.java @@ -34,6 +34,7 @@ import au.com.integradev.delphi.executor.DelphiMasterExecutor; import au.com.integradev.delphi.msbuild.DelphiProjectHelper; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -42,7 +43,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.fs.TextRange; +import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.sensor.SensorDescriptor; +import org.sonar.api.batch.sensor.internal.SensorContextTester; class DelphiSensorTest { private final DelphiMasterExecutor executor = mock(DelphiMasterExecutor.class); @@ -84,15 +88,53 @@ void setup() throws IOException { + "implementation\n" + "end."); + setupFile("unit SourceFile;\ninterface\nimplementation\nend."); + when(delphiProjectHelper.standardLibraryPath()).thenReturn(standardLibraryPath); + } + + private void setupFile(String content) { Path sourceFilePath = baseDir.resolve("SourceFile.pas"); - Files.writeString(sourceFilePath, "unit SourceFile;\ninterface\nimplementation\nend."); - InputFile inputFile = mock(InputFile.class); - when(inputFile.uri()).thenReturn(sourceFilePath.toUri()); + try { + Files.writeString(sourceFilePath, content); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + + InputFile inputFile = + TestInputFileBuilder.create("moduleKey", baseDir.toFile(), sourceFilePath.toFile()) + .setContents(content) + .setLanguage(Delphi.KEY) + .setType(InputFile.Type.MAIN) + .build(); when(delphiProjectHelper.inputFiles()).thenReturn(List.of(inputFile)); when(delphiProjectHelper.getFile(anyString())).thenReturn(inputFile); - when(delphiProjectHelper.standardLibraryPath()).thenReturn(standardLibraryPath); + } + + private void assertParsingErrorIssue(int expectedLine, String expectedMessage) { + SensorContextTester context = SensorContextTester.create(baseDir); + + sensor.execute(context); + + assertThat(context.allIssues()) + .hasSize(1) + .element(0) + .satisfies( + issue -> { + assertThat(issue.ruleKey().repository()).isEqualTo("community-delphi"); + assertThat(issue.ruleKey().rule()).isEqualTo("ParsingError"); + assertThat(issue.primaryLocation().message()).isEqualTo(expectedMessage); + + TextRange position = issue.primaryLocation().textRange(); + if (expectedLine == 0) { + assertThat(position).isNull(); + } else { + assertThat(position).isNotNull(); + assertThat(position.start().line()).isEqualTo(expectedLine); + assertThat(position.end().line()).isEqualTo(expectedLine); + } + }); } @AfterEach @@ -143,4 +185,23 @@ void testWhenShouldExecuteOnProjectReturnsTrueThenExecutorIsCalled() { verify(executor, times(1)).execute(any(), any()); } + + @Test + void testFileWithLexerErrorRaisesParsingErrorIssue() { + setupFile("\n\n'unterminated string literal"); + assertParsingErrorIssue( + 3, "Parse error (line 3:28 mismatched character '' expecting ''')"); + } + + @Test + void testFileWithParserErrorRaisesParsingErrorIssue() { + setupFile("\n\n\n\n;"); + assertParsingErrorIssue(5, "Parse error (line 5:0 no viable alternative at input ';')"); + } + + @Test + void testEmptyFileRaisesParsingErrorIssue() { + setupFile(""); + assertParsingErrorIssue(0, "Parse error (Empty files are not allowed)"); + } }