diff --git a/CHANGELOG.md b/CHANGELOG.md index 771c6cd4d..31a514bf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Include the global browsing path in unit import resolution. +- Reprioritize the analysis search path in the following order (highest to lowest): + - Analysis source files (`sonar.sources`) + - Referenced project files (`DCCReference`) + - Search path (`DCC_UnitSearchPath`) + - Debugger source path (`Debugger_DebugSourcePath`) + - Library path (`DelphiLibraryPath`/`DelphiTranslatedLibraryPath`) + - Browsing path (`DelphiBrowsingPath`) + - Standard library + ## [1.12.2] - 2025-01-06 ### Fixed diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProject.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProject.java index a97678445..e0d8b422f 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProject.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProject.java @@ -38,5 +38,9 @@ public interface DelphiProject { List getDebugSourceDirectories(); + List getLibraryPathDirectories(); + + List getBrowsingPathDirectories(); + Map getUnitAliases(); } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectHelper.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectHelper.java index df7d24d80..b5d4da91f 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectHelper.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectHelper.java @@ -69,6 +69,8 @@ public class DelphiProjectHelper { private final CompilerVersion compilerVersion; private final List searchDirectories; private final List debugSourceDirectories; + private final List libraryPathDirectories; + private final List browsingPathDirectories; private final List referencedFiles; private final Set conditionalDefines; private final Set unitScopeNames; @@ -93,6 +95,8 @@ public DelphiProjectHelper( this.compilerVersion = getCompilerVersionFromSettings(); this.searchDirectories = getSearchDirectoriesFromSettings(); this.debugSourceDirectories = new ArrayList<>(); + this.libraryPathDirectories = new ArrayList<>(); + this.browsingPathDirectories = new ArrayList<>(); this.referencedFiles = new ArrayList<>(); this.conditionalDefines = getPredefinedConditionalDefines(); this.unitScopeNames = getSetFromSettings(DelphiProperties.UNIT_SCOPE_NAMES_KEY); @@ -201,6 +205,8 @@ private void indexProjects() { for (DelphiProject project : projects) { searchDirectories.addAll(project.getSearchDirectories()); debugSourceDirectories.addAll(project.getDebugSourceDirectories()); + libraryPathDirectories.addAll(project.getLibraryPathDirectories()); + browsingPathDirectories.addAll(project.getBrowsingPathDirectories()); conditionalDefines.addAll(project.getConditionalDefines()); referencedFiles.addAll(project.getSourceFiles()); unitScopeNames.addAll(project.getUnitScopeNames()); @@ -310,13 +316,33 @@ public List getSearchDirectories() { /** * Gets the debug source directories specified in project files * - * @return List of debug source directorie + * @return List of debug source directories */ public List getDebugSourceDirectories() { indexProjects(); return debugSourceDirectories; } + /** + * Gets the library path directories specified in project files + * + * @return List of library path directories + */ + public List getLibraryPathDirectories() { + indexProjects(); + return libraryPathDirectories; + } + + /** + * Gets the browsing path directories specified in project files + * + * @return List of browsing path directories + */ + public List getBrowsingPathDirectories() { + indexProjects(); + return browsingPathDirectories; + } + /** * Gets the set of conditional defines specified in settings and project files * diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectParser.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectParser.java index 2b6d5ecab..c24ed3ecc 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectParser.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/DelphiProjectParser.java @@ -66,6 +66,8 @@ public DelphiProject parse() { project.setUnitScopeNames(createUnitScopeNames(result.getProperties())); project.setSearchDirectories(createSearchDirectories(dprojDirectory, result.getProperties())); project.setDebugSourceDirectories(createDebugSourceDirectories(result.getProperties())); + project.setLibraryPath(createLibraryPathDirectories(result.getProperties())); + project.setBrowsingPath(createBrowsingPathDirectories(result.getProperties())); project.setUnitAliases(createUnitAliases(result.getProperties())); project.setSourceFiles(result.getSourceFiles()); @@ -81,31 +83,29 @@ private static Set createUnitScopeNames(ProjectProperties properties) { } private List createSearchDirectories(Path dprojDirectory, ProjectProperties properties) { - /* - We manually append the library paths here, even though it's not strictly correct to do so. + List result = new ArrayList<>(); - CodeGear.Delphi.Targets appends the library paths to DCC_UnitSearchPath to create a new - property called UnitSearchPath, which then gets passed through to the compiler. + result.add(dprojDirectory); + result.addAll(createPathList(properties, "DCC_UnitSearchPath")); - However, there are some good reasons not to just read the UnitSearchPath property: + return Collections.unmodifiableList(result); + } - - It would tie us to an implementation detail of the MSBuild glue in CodeGear.Delphi.Targets. - - If the UnitSearchPath property were ever renamed, we'd fall out of compatibility. - - Relying on CodeGear.Delphi.Targets details would require us to mock it up in testing. - */ + private List createDebugSourceDirectories(ProjectProperties properties) { + return createPathList(properties, "Debugger_DebugSourcePath"); + } - List allPaths = new ArrayList<>(); + private List createLibraryPathDirectories(ProjectProperties properties) { + List result = new ArrayList<>(); - allPaths.add(dprojDirectory); - allPaths.addAll(createPathList(properties, "DCC_UnitSearchPath")); - allPaths.addAll(createPathList(properties, "DelphiLibraryPath", false)); - allPaths.addAll(createPathList(properties, "DelphiTranslatedLibraryPath", false)); + result.addAll(createPathList(properties, "DelphiLibraryPath", false)); + result.addAll(createPathList(properties, "DelphiTranslatedLibraryPath", false)); - return Collections.unmodifiableList(allPaths); + return Collections.unmodifiableList(result); } - private List createDebugSourceDirectories(ProjectProperties properties) { - return createPathList(properties, "Debugger_DebugSourcePath"); + private List createBrowsingPathDirectories(ProjectProperties properties) { + return createPathList(properties, "DelphiBrowsingPath", false); } private List createPathList(ProjectProperties properties, String propertyName) { @@ -176,6 +176,8 @@ private static class DelphiProjectImpl implements DelphiProject { private List sourceFiles = Collections.emptyList(); private List searchDirectories = Collections.emptyList(); private List debugSourceDirectories = Collections.emptyList(); + private List libraryPathDirectories = Collections.emptyList(); + private List browsingPathDirectories = Collections.emptyList(); private Map unitAliases = Collections.emptyMap(); private void setDefinitions(Set definitions) { @@ -199,6 +201,14 @@ private void setDebugSourceDirectories(List debugSourceDirectories) { this.debugSourceDirectories = List.copyOf(debugSourceDirectories); } + private void setLibraryPath(List libraryPathDirectories) { + this.libraryPathDirectories = List.copyOf(libraryPathDirectories); + } + + private void setBrowsingPath(List browsingPathDirectories) { + this.browsingPathDirectories = List.copyOf(browsingPathDirectories); + } + private void setUnitAliases(Map unitAliases) { this.unitAliases = ImmutableSortedMap.copyOf(unitAliases, String.CASE_INSENSITIVE_ORDER); } @@ -228,6 +238,16 @@ public List getDebugSourceDirectories() { return debugSourceDirectories; } + @Override + public List getLibraryPathDirectories() { + return libraryPathDirectories; + } + + @Override + public List getBrowsingPathDirectories() { + return browsingPathDirectories; + } + @Override public Map getUnitAliases() { return unitAliases; diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java index 702469d08..cc9e1028c 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/SymbolTableBuilder.java @@ -185,7 +185,7 @@ private static boolean isPasFile(Path path) { } private void createUnitData(Path unitPath, boolean isSourceFile) { - if (unitPaths.add(unitPath) || isSourceFile) { + if (unitPaths.add(unitPath)) { String unitName = FilenameUtils.getBaseName(unitPath.toString()); UnitData unitData = new UnitData(unitPath, isSourceFile); @@ -459,10 +459,11 @@ public SymbolTable build() { throw new SymbolTableConstructionException("typeFactory was not supplied."); } - processStandardLibrarySearchPaths(); - searchPath.getRootDirectories().forEach(this::processSearchPath); - referencedFiles.forEach(file -> this.createUnitData(file, false)); sourceFiles.forEach(file -> this.createUnitData(file, true)); + referencedFiles.forEach(file -> this.createUnitData(file, false)); + searchPath.getRootDirectories().forEach(this::processSearchPath); + + processStandardLibrarySearchPaths(); ProgressReport progressReport = new ProgressReport( diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/msbuild/DelphiProjectParserTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/msbuild/DelphiProjectParserTest.java index 0c2d0d093..5abf85d46 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/msbuild/DelphiProjectParserTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/msbuild/DelphiProjectParserTest.java @@ -57,6 +57,9 @@ class DelphiProjectParserTest { private static final String LIBRARY_PATH_PROJECT = "/au/com/integradev/delphi/msbuild/LibraryPath.dproj"; + private static final String BROWSING_PATH_PROJECT = + "/au/com/integradev/delphi/msbuild/BrowsingPath.dproj"; + private EnvironmentVariableProvider environmentVariableProvider; private Path environmentProj; @@ -158,14 +161,20 @@ void testBadSourceFileProjectShouldContainValidSourceFiles() { } @Test - void testLibraryPathShouldBeAppendedToSearchDirectories() { + void testLibraryPathProject() { DelphiProject project = parse(LIBRARY_PATH_PROJECT); - assertThat(project.getSearchDirectories()) + assertThat(project.getLibraryPathDirectories()) .containsExactly( - DelphiUtils.getResource("/au/com/integradev/delphi/msbuild").toPath(), DelphiUtils.getResource("/au/com/integradev/delphi").toPath(), - DelphiUtils.getResource("/au/com/integradev").toPath(), - DelphiUtils.getResource("/au/com").toPath()); + DelphiUtils.getResource("/au/com/integradev").toPath()); + } + + @Test + void testBrowsingPathProject() { + DelphiProject project = parse(BROWSING_PATH_PROJECT); + + assertThat(project.getBrowsingPathDirectories()) + .containsExactly(DelphiUtils.getResource("/au/com/integradev/delphi").toPath()); } } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/SymbolTableBuilderTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/SymbolTableBuilderTest.java index 1bc0809f9..e546b2ce3 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/SymbolTableBuilderTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/symbol/SymbolTableBuilderTest.java @@ -38,6 +38,8 @@ import java.nio.file.spi.FileSystemProvider; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; @@ -157,6 +159,95 @@ void testStandardLibrarySearchPathShouldExcludeToolsUnits( assertThat(symbolTable.getUnitByPath(excludedPath.toString())).isNull(); } + @Test + void testSonarSourcesArePrioritizedOverReferencedFiles( + @TempDir Path standardLibraryPath, + @TempDir Path referencedFilesPath, + @TempDir Path sourceFilesPath) + throws IOException { + createStandardLibrary(standardLibraryPath); + createStandardLibrary(referencedFilesPath); + createStandardLibrary(sourceFilesPath); + + List referencedFiles; + try (Stream referencedFilesStream = Files.list(referencedFilesPath)) { + referencedFiles = referencedFilesStream.collect(Collectors.toUnmodifiableList()); + } + + List sourceFiles; + try (Stream sourceFilesStream = Files.list(sourceFilesPath)) { + sourceFiles = sourceFilesStream.collect(Collectors.toUnmodifiableList()); + } + + SymbolTable symbolTable = + SymbolTable.builder() + .preprocessorFactory(new DelphiPreprocessorFactory(Platform.WINDOWS)) + .typeFactory(TypeFactoryUtils.defaultFactory()) + .standardLibraryPath(standardLibraryPath) + .referencedFiles(referencedFiles) + .sourceFiles(sourceFiles) + .build(); + + assertThat(symbolTable.getUnitByPath(standardLibraryPath.resolve("SysInit.pas").toString())) + .isNull(); + assertThat(symbolTable.getUnitByPath(referencedFilesPath.resolve("SysInit.pas").toString())) + .isNull(); + assertThat(symbolTable.getUnitByPath(sourceFilesPath.resolve("SysInit.pas").toString())) + .isNotNull(); + } + + @Test + void testReferencedFilesArePrioritizedOverSearchPath( + @TempDir Path standardLibraryPath, + @TempDir Path searchPathRoot, + @TempDir Path referencedFilesPath) + throws IOException { + createStandardLibrary(standardLibraryPath); + createStandardLibrary(searchPathRoot); + createStandardLibrary(referencedFilesPath); + + List referencedFiles; + try (Stream referencedFilesStream = Files.list(referencedFilesPath)) { + referencedFiles = referencedFilesStream.collect(Collectors.toUnmodifiableList()); + } + + SymbolTable symbolTable = + SymbolTable.builder() + .preprocessorFactory(new DelphiPreprocessorFactory(Platform.WINDOWS)) + .typeFactory(TypeFactoryUtils.defaultFactory()) + .standardLibraryPath(standardLibraryPath) + .searchPath(SearchPath.create(List.of(searchPathRoot))) + .referencedFiles(referencedFiles) + .build(); + + assertThat(symbolTable.getUnitByPath(standardLibraryPath.resolve("SysInit.pas").toString())) + .isNull(); + assertThat(symbolTable.getUnitByPath(searchPathRoot.resolve("SysInit.pas").toString())) + .isNull(); + assertThat(symbolTable.getUnitByPath(referencedFilesPath.resolve("SysInit.pas").toString())) + .isNotNull(); + } + + @Test + void testSearchPathIsPrioritizedOverStandardLibrary( + @TempDir Path standardLibraryPath, @TempDir Path searchPathRoot) throws IOException { + createStandardLibrary(standardLibraryPath); + createStandardLibrary(searchPathRoot); + + SymbolTable symbolTable = + SymbolTable.builder() + .preprocessorFactory(new DelphiPreprocessorFactory(Platform.WINDOWS)) + .typeFactory(TypeFactoryUtils.defaultFactory()) + .standardLibraryPath(standardLibraryPath) + .searchPath(SearchPath.create(List.of(searchPathRoot))) + .build(); + + assertThat(symbolTable.getUnitByPath(standardLibraryPath.resolve("SysInit.pas").toString())) + .isNull(); + assertThat(symbolTable.getUnitByPath(searchPathRoot.resolve("SysInit.pas").toString())) + .isNotNull(); + } + @ParameterizedTest @ValueSource(strings = {"#$@", "unit ErrorUnit;"}) void testRecognitionErrorInImportShouldNotThrow( diff --git a/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/BrowsingPath.dproj b/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/BrowsingPath.dproj new file mode 100644 index 000000000..5799f38c2 --- /dev/null +++ b/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/BrowsingPath.dproj @@ -0,0 +1,5 @@ + + + ../ + + diff --git a/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/LibraryPath.dproj b/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/LibraryPath.dproj index 79ab2438e..68e0024ef 100644 --- a/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/LibraryPath.dproj +++ b/delphi-frontend/src/test/resources/au/com/integradev/delphi/msbuild/LibraryPath.dproj @@ -1,7 +1,6 @@  - ../../ - ../../../ - ../ + ../ + ../../ 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 7264699a1..38910bb12 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 @@ -99,10 +99,7 @@ private void executeOnFiles(SensorContext sensorContext) { Iterable inputFiles = delphiProjectHelper.inputFiles(); List sourceFiles = inputFilesToPaths(inputFiles); List referencedFiles = delphiProjectHelper.getReferencedFiles(); - List searchPathDirectories = new ArrayList<>(); - searchPathDirectories.addAll(delphiProjectHelper.getSearchDirectories()); - searchPathDirectories.addAll(delphiProjectHelper.getDebugSourceDirectories()); - SearchPath searchPath = SearchPath.create(searchPathDirectories); + SearchPath searchPath = createSearchPath(); SymbolTable symbolTable = SymbolTable.builder() @@ -153,6 +150,33 @@ private void executeOnFiles(SensorContext sensorContext) { } } + private SearchPath createSearchPath() { + /* + CodeGear.Delphi.Targets appends the library paths to DCC_UnitSearchPath to create a new + property called UnitSearchPath, which then gets passed through to the compiler. + If we were reading UnitSearchPath directly instead of DCC_UnitSearchPath, we could avoid + manually appending the library path here. + + There's a major benefit though, as the current approach allows us to prioritize the debug + source paths in between DCC_UnitSearchPath and DelphiLibraryPath in the search path. + From the perspective of a static analysis tool, this is the more correct ordering since it's + common for the debug source path to contain corresponding source files for DCUs in the search + path. These debug sources are more local to the project than the library path and should be + prioritized higher. + + Some more reasons not to just read the UnitSearchPath property... + - It would tie us to an implementation detail of the MSBuild glue in CodeGear.Delphi.Targets. + - If the UnitSearchPath property were ever renamed, we'd fall out of compatibility. + - Relying on CodeGear.Delphi.Targets details would require us to mock it up in testing. + */ + List searchPathDirectories = new ArrayList<>(); + searchPathDirectories.addAll(delphiProjectHelper.getSearchDirectories()); + searchPathDirectories.addAll(delphiProjectHelper.getDebugSourceDirectories()); + searchPathDirectories.addAll(delphiProjectHelper.getLibraryPathDirectories()); + searchPathDirectories.addAll(delphiProjectHelper.getBrowsingPathDirectories()); + return SearchPath.create(searchPathDirectories); + } + private boolean shouldExecuteOnProject() { return delphiProjectHelper.shouldExecuteOnProject(); }