From d58245d0fa2ad19a2223e33eb4dea721decef0e9 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Thu, 17 Sep 2026 14:57:38 -0400 Subject: [PATCH] feat(ids): a module's can:// id names the module, not where it was checked out A module id gains a segment naming the module as its own manifest declares it -- Maven's , Gradle's rootProject.name -- between the language segment and the path within that module: can://daytrader/java/daytrader-web-service/src/main/java/.../Bean.java So pointing --input at one service repo names that service in the id, with no staging directory and without pointing --input at a parent that sweeps in sibling repos. The coordinate is read from the manifest rather than taken from the directory name because a declared name survives a rename, a relocated checkout and a differently-laid-out CI workspace, where a path-derived segment would make every id a function of where the tree happens to sit on disk. The symbol_table key does NOT change. It stays the real --input-relative path: uniqueness is the key's job, a path is unique by construction where a coordinate is not, and with no path field on the module object the key is also the only thing that still locates a file on disk. The key is therefore no longer the tail of the id, and nothing may assume it is -- L3WalaOverlays recovered the application id by stripping the key off a module id, so it is passed the real one instead. The same non-uniqueness reaches the id, which is the join key for call_graph endpoints. A coordinate claimed by two module directories is applied to neither, with a warning naming the competitors; vendored duplicates are exactly where this bites, and applying the prefix there would merge two distinct callables onto one node. Without that rule the change would regress "ids are distinct by construction", the property it was chosen for. A module with no readable coordinate keeps precisely the id it had before, so a loose source tree is unaffected. Spec: codellm-devkit/.github docs/design/specs/2026-09-17-manifest-derived-module-prefix.md --- src/main/java/com/ibm/cldk/CodeAnalyzer.java | 4 +- .../java/com/ibm/cldk/L3WalaOverlays.java | 31 +--- .../ibm/cldk/artifacts/ModuleCoordinates.java | 121 +++++++++++++ .../syntactic_analysis/L1BuildContext.java | 26 ++- .../cldk/syntactic_analysis/L1Extractor.java | 20 ++- .../syntactic_analysis/ModulePrefixes.java | 160 ++++++++++++++++++ .../java/com/ibm/cldk/L3WalaOverlaysTest.java | 33 ---- .../cldk/artifacts/ModuleCoordinatesTest.java | 71 ++++++++ .../ibm/cldk/schema/L2CallGraphGateTest.java | 18 +- .../L1BuildContextTest.java | 41 +++++ .../ModulePrefixesTest.java | 112 ++++++++++++ 11 files changed, 571 insertions(+), 66 deletions(-) create mode 100644 src/main/java/com/ibm/cldk/artifacts/ModuleCoordinates.java create mode 100644 src/main/java/com/ibm/cldk/syntactic_analysis/ModulePrefixes.java delete mode 100644 src/test/java/com/ibm/cldk/L3WalaOverlaysTest.java create mode 100644 src/test/java/com/ibm/cldk/artifacts/ModuleCoordinatesTest.java create mode 100644 src/test/java/com/ibm/cldk/syntactic_analysis/L1BuildContextTest.java create mode 100644 src/test/java/com/ibm/cldk/syntactic_analysis/ModulePrefixesTest.java diff --git a/src/main/java/com/ibm/cldk/CodeAnalyzer.java b/src/main/java/com/ibm/cldk/CodeAnalyzer.java index bc57b87b..8bc7f21e 100644 --- a/src/main/java/com/ibm/cldk/CodeAnalyzer.java +++ b/src/main/java/com/ibm/cldk/CodeAnalyzer.java @@ -31,6 +31,7 @@ import com.ibm.cldk.neo4j.BoltConfig; import com.ibm.cldk.neo4j.Neo4jEmitter; import com.ibm.cldk.schema.Analysis; +import com.ibm.cldk.schema.CanId; import com.ibm.cldk.schema.JArtifact; import com.ibm.cldk.schema.JDependency; import com.ibm.cldk.schema.JEntrypointReport; @@ -531,7 +532,8 @@ private void analyzeV2() throws Exception { } // Apply WALA L3 overlays while the dependency jars are still live (PDG/CFG need class files). if (wala != null) { - L3WalaOverlays.apply(wala, input, modules, graphFieldDepth); + L3WalaOverlays.apply(wala, input, modules, graphFieldDepth, + CanId.applicationId(application)); } // L4: the semantic ddg needs a WALA build regardless of --l3-engine, so build it here (or // reuse the instance --l3-engine wala already built above) while the jars are still live. diff --git a/src/main/java/com/ibm/cldk/L3WalaOverlays.java b/src/main/java/com/ibm/cldk/L3WalaOverlays.java index ca4eae0a..7edf4905 100644 --- a/src/main/java/com/ibm/cldk/L3WalaOverlays.java +++ b/src/main/java/com/ibm/cldk/L3WalaOverlays.java @@ -66,20 +66,21 @@ private L3WalaOverlays() {} * @param input the project root directory * @param modules the L1 module map (mutated in place: cfg/cdg/ddg are set on callables) * @param fieldDepth the DDG access-path bound k ({@code --graph-field-depth}) + * @param applicationId the {@code can://} root, passed in rather than reverse-engineered + * from a module id — a module id no longer ends with its {@code symbol_table} key, since the + * id additionally carries the module's declared coordinate (see {@code ModulePrefixes}) */ public static void apply( WalaAnalysis wala, String input, Map modules, - int fieldDepth) { + int fieldDepth, + String applicationId) { if (modules.isEmpty()) { return; } - // Derive the applicationId from any module in the map. - String applicationId = deriveApplicationId(modules); - // Build a binary-type-name → (moduleKey, JType) index. Map typeIndex = buildTypeIndex(modules); @@ -362,28 +363,6 @@ private static TypeDeclaration findTypeDecl( return current; } - // ----- applicationId derivation ------------------------------------------------------------- - - /** - * Derives the {@code can://} applicationId from the first entry in {@code modules}. - * The module id has the form {@code applicationId/normalizedFileKey}, so strip the suffix. - */ - private static String deriveApplicationId(Map modules) { - Map.Entry first = modules.entrySet().iterator().next(); - String moduleId = first.getValue().getId(); - if (moduleId == null) { - return CanId.applicationId("unknown"); - } - String normalizedFileKey = first.getKey().replace("\\", "/").replaceFirst("^[./]+", ""); - int sep = moduleId.lastIndexOf("/" + normalizedFileKey); - if (sep > 0) { - return moduleId.substring(0, sep); - } - // Fallback: trim the last slash-delimited segment matching the key. - int last = moduleId.lastIndexOf('/'); - return last > 0 ? moduleId.substring(0, last) : moduleId; - } - // ----- inner types -------------------------------------------------------------------------- /** One WALA method successfully joined to its L1 callable and re-parsed body block. */ diff --git a/src/main/java/com/ibm/cldk/artifacts/ModuleCoordinates.java b/src/main/java/com/ibm/cldk/artifacts/ModuleCoordinates.java new file mode 100644 index 00000000..9bdc9a23 --- /dev/null +++ b/src/main/java/com/ibm/cldk/artifacts/ModuleCoordinates.java @@ -0,0 +1,121 @@ +package com.ibm.cldk.artifacts; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.ParserConfigurationException; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.SAXException; + +/** + * The name a build module gives itself — Maven's {@code }, Gradle's + * {@code rootProject.name}. + * + *

This is the coordinate the {@code can://} module segment is built from, and the reason it is + * read from the manifest rather than taken from the directory name: a declared coordinate survives + * a rename, a relocated checkout and a differently-laid-out CI workspace, where a path-derived + * segment would make every id a function of where the tree happens to sit on disk. + * + *

Every method is total — a missing, malformed or hostile manifest yields + * {@link Optional#empty()} rather than throwing, because a module with no readable coordinate is a + * module that simply gets no prefix, not an analysis failure. + */ +public final class ModuleCoordinates { + + private ModuleCoordinates() {} + + /** {@code rootProject.name = 'x'} / {@code rootProject.name = "x"}, Groovy and Kotlin alike. */ + private static final Pattern GRADLE_ROOT_NAME = + Pattern.compile("rootProject\\s*\\.\\s*name\\s*=\\s*[\"']([^\"']+)[\"']"); + + /** + * The coordinate declared by the manifest in {@code directory}, or empty when it declares none. + * + *

Maven is consulted first because a directory carrying both is a Maven module with a Gradle + * build bolted alongside far more often than the reverse. + */ + public static Optional nameOf(Path directory) { + Path pom = directory.resolve("pom.xml"); + if (Files.isRegularFile(pom)) { + return mavenArtifactId(pom); + } + for (String settings : new String[] {"settings.gradle", "settings.gradle.kts"}) { + Path path = directory.resolve(settings); + if (Files.isRegularFile(path)) { + return gradleRootName(path); + } + } + // A bare build.gradle deliberately yields nothing. Gradle's project name lives in the + // settings file, and falling back to the directory name here would quietly reintroduce the + // location-dependence this class exists to avoid. + return Optional.empty(); + } + + /** + * {@code /project/artifactId} — a direct child of the root element only. + * + *

Descending would find {@code } and every + * {@code }, each of which names a different module. A pom that inherits + * its artifactId from a parent declares none of its own and correctly yields empty. + */ + private static Optional mavenArtifactId(Path pom) { + try { + DocumentBuilder builder = + ManifestParsers.newSecureDocumentBuilderFactory().newDocumentBuilder(); + Element root = builder.parse(new ByteArrayInputStream(Files.readAllBytes(pom))) + .getDocumentElement(); + if (root == null) { + return Optional.empty(); + } + NodeList children = root.getChildNodes(); + for (int i = 0; i < children.getLength(); i++) { + Node child = children.item(i); + if (child.getNodeType() != Node.ELEMENT_NODE) { + continue; + } + String name = child.getLocalName() != null ? child.getLocalName() : child.getNodeName(); + if ("artifactId".equals(name)) { + return nonBlank(child.getTextContent()); + } + } + return Optional.empty(); + } catch (ParserConfigurationException | SAXException | IOException e) { + // Exactly the checked contract of the DOM helpers plus the read. A pom declaring a + // DOCTYPE arrives here as a SAXException, which is the categorical refusal + // ManifestParsers documents for manifests, and it means "no coordinate", not "fail". + return Optional.empty(); + } + } + + private static Optional gradleRootName(Path settings) { + try { + Matcher matcher = + GRADLE_ROOT_NAME.matcher(Files.readString(settings, StandardCharsets.UTF_8)); + return matcher.find() ? nonBlank(matcher.group(1)) : Optional.empty(); + } catch (IOException | RuntimeException e) { + // RuntimeException covers the unreadable-as-UTF-8 case (MalformedInputException arrives + // wrapped) and an oversized file; neither is an analysis failure. + return Optional.empty(); + } + } + + private static Optional nonBlank(String text) { + if (text == null || text.isBlank()) { + return Optional.empty(); + } + String trimmed = text.trim(); + // A coordinate is one path segment of an id. One carrying a separator would silently add a + // level to the containment path, so it is refused rather than sanitized. + return trimmed.indexOf('/') >= 0 || trimmed.indexOf('\\') >= 0 + ? Optional.empty() + : Optional.of(trimmed); + } +} diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/L1BuildContext.java b/src/main/java/com/ibm/cldk/syntactic_analysis/L1BuildContext.java index f8a9f7dc..a38801d4 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/L1BuildContext.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/L1BuildContext.java @@ -30,6 +30,20 @@ public final class L1BuildContext { private final String applicationId; private final String fileKey; + + /** + * The path segment the {@code can://} id is built from, which is NOT always {@link #fileKey}. + * + *

The key is the real {@code --input}-relative path, because uniqueness is the key's job and a + * path is unique by construction. The id path additionally carries the module's declared + * coordinate when {@link ModulePrefixes} resolved an unambiguous one + * ({@code /}), so an id names the module a file belongs to + * rather than where the tree happened to be checked out. The two coincide whenever no coordinate + * applies, which is why every convenience constructor here defaults one to the other — but + * nothing downstream may assume the key is the tail of the id. + */ + private final String idPath; + private final String source; /** The requested analysis level; the L3 dataflow pass runs at parse time when this is {@code >= 3}. */ @@ -78,18 +92,26 @@ public L1BuildContext(String applicationId, String fileKey, String source, int a public L1BuildContext(String applicationId, String fileKey, String source, int analysisLevel, int graphFieldDepth, String l3Engine, JEntrypointReport entrypointReport) { + this(applicationId, fileKey, fileKey, source, analysisLevel, graphFieldDepth, l3Engine, + entrypointReport); + } + + public L1BuildContext(String applicationId, String fileKey, String idPath, String source, + int analysisLevel, int graphFieldDepth, String l3Engine, + JEntrypointReport entrypointReport) { this.entrypointReport = entrypointReport != null ? entrypointReport : new JEntrypointReport(); this.applicationId = applicationId; this.fileKey = fileKey; + this.idPath = idPath != null ? idPath : fileKey; this.source = source; this.analysisLevel = analysisLevel; this.graphFieldDepth = graphFieldDepth; this.l3Engine = l3Engine != null ? l3Engine.toLowerCase(java.util.Locale.ROOT) : "ast"; } - /** The {@code can:///java/} id for this module. */ + /** The {@code can:///java/} id for this module. */ public String moduleId() { - return CanId.moduleId(applicationId, fileKey); + return CanId.moduleId(applicationId, idPath); } /** diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/L1Extractor.java b/src/main/java/com/ibm/cldk/syntactic_analysis/L1Extractor.java index ecf72e8e..4b3daf8a 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/L1Extractor.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/L1Extractor.java @@ -143,6 +143,22 @@ public static Map extractAll( String applicationId = CanId.applicationId(appName); JavaParser parser = new JavaParser(config); int reused = 0; + + // Resolved over the WHOLE file set before any module is built: the uniqueness rule needs to + // see every claimant of a coordinate before it can decide whether to apply it. + List allFiles = new ArrayList<>(); + for (SourceRoot sourceRoot : sourceRoots) { + allFiles.addAll(javaFilesUnder(sourceRoot.getRoot())); + } + Path analysisRoot = projectRoot; + ModulePrefixes prefixes = ModulePrefixes.resolve(analysisRoot, allFiles); + prefixes.contested().forEach((coordinate, dirs) -> Log.warn( + "Module coordinate '" + coordinate + "' is declared by " + dirs.size() + + " modules (" + dirs.stream().map(d -> analysisRoot.relativize(d).toString()) + .collect(java.util.stream.Collectors.joining(", ")) + + "); ids for these keep their plain relative path, since a shared coordinate" + + " would collide them")); + for (SourceRoot sourceRoot : sourceRoots) { for (Path path : javaFilesUnder(sourceRoot.getRoot())) { String fileKey = fileKey(projectRoot, path); @@ -150,8 +166,8 @@ public static Map extractAll( // real file, byte for byte. String source = Files.readString(path, StandardCharsets.UTF_8); L1BuildContext ctx = new L1BuildContext( - applicationId, fileKey, source, analysisLevel, graphFieldDepth, l3Engine, - entrypointReport); + applicationId, fileKey, prefixes.idPath(path), source, analysisLevel, + graphFieldDepth, l3Engine, entrypointReport); // Reuse the cached module when the file is byte-for-byte what it was last time. This // skips the parse as well as the build, which is where the cost is. diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/ModulePrefixes.java b/src/main/java/com/ibm/cldk/syntactic_analysis/ModulePrefixes.java new file mode 100644 index 00000000..137d52f5 --- /dev/null +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/ModulePrefixes.java @@ -0,0 +1,160 @@ +package com.ibm.cldk.syntactic_analysis; + +import com.ibm.cldk.artifacts.ModuleCoordinates; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; + +/** + * Resolves the build module each source file belongs to, and the {@code can://} id path that follows + * from it: {@code /}, or the bare + * {@code --input}-relative path when no coordinate applies. + * + *

This shapes the id only. The {@code symbol_table} key stays the real + * {@code --input}-relative path, because the key's job is uniqueness and a path is unique by + * construction where a coordinate is not — and because the key and the id are the only things in + * the output that locate a file on disk. + * + *

The coordinate is applied only when exactly one module directory claims it. The id is + * the join key for call-graph endpoints, so a coordinate claimed by two directories would merge two + * distinct callables onto one node — the same silent loss, one level up. Vendored duplicates are + * where this bites: several services each building one shared internal library under the same module + * name. A contested coordinate is therefore applied to none of its claimants, which keeps "ids are + * distinct by construction" true rather than trading it away for a nicer spelling. + * + *

The search for a manifest walks up from each file and stops at {@code --input}, so a + * manifest above the analysis root is invisible and cannot be mistaken for the enclosing module of a + * tree that merely happens to sit inside one. + */ +public final class ModulePrefixes { + + private final Path inputRoot; + + /** File → its module directory, for files whose coordinate survived the uniqueness check. */ + private final Map moduleDirByFile; + + /** Module directory → the coordinate applied to it. */ + private final Map coordinateByModuleDir; + + /** Coordinate → the competing module directories that claimed it, in path order. */ + private final Map> contested; + + private ModulePrefixes( + Path inputRoot, + Map moduleDirByFile, + Map coordinateByModuleDir, + Map> contested) { + this.inputRoot = inputRoot; + this.moduleDirByFile = moduleDirByFile; + this.coordinateByModuleDir = coordinateByModuleDir; + this.contested = contested; + } + + /** + * Resolve every file's module, then drop any coordinate claimed by more than one module + * directory. + * + * @param inputRoot the analysis root; the manifest search never looks above it + * @param files the source files being analysed + */ + public static ModulePrefixes resolve(Path inputRoot, Collection files) { + Path root = inputRoot.toAbsolutePath().normalize(); + + // Memoized per directory: a module of a few hundred files would otherwise re-read and + // re-parse its pom once per file. + Map> moduleDirByDirectory = new HashMap<>(); + Map coordinateByModuleDir = new HashMap<>(); + Map moduleDirByFile = new LinkedHashMap<>(); + + for (Path file : files) { + Path absolute = file.toAbsolutePath().normalize(); + Path directory = absolute.getParent(); + if (directory == null) { + continue; + } + Optional moduleDir = moduleDirByDirectory.computeIfAbsent( + directory, d -> nearestModuleDir(d, root, coordinateByModuleDir)); + moduleDir.ifPresent(dir -> moduleDirByFile.put(absolute, dir)); + } + + // One coordinate, two or more directories: apply it to neither. + Map> claimants = new TreeMap<>(); + coordinateByModuleDir.forEach( + (dir, coordinate) -> claimants.computeIfAbsent(coordinate, c -> new TreeSet<>()).add(dir)); + + Map> contested = new LinkedHashMap<>(); + claimants.forEach((coordinate, dirs) -> { + if (dirs.size() > 1) { + contested.put(coordinate, List.copyOf(dirs)); + dirs.forEach(coordinateByModuleDir::remove); + } + }); + moduleDirByFile.values().removeIf(dir -> !coordinateByModuleDir.containsKey(dir)); + + return new ModulePrefixes( + root, + Collections.unmodifiableMap(moduleDirByFile), + Collections.unmodifiableMap(coordinateByModuleDir), + Collections.unmodifiableMap(contested)); + } + + /** + * Walk up from {@code directory} to {@code root} inclusive; the first directory declaring a + * coordinate is the file's module. Nearest wins, so a reactor child carries its own coordinate + * rather than the aggregator's. + */ + private static Optional nearestModuleDir( + Path directory, Path root, Map coordinateByModuleDir) { + for (Path current = directory; + current != null && current.startsWith(root); + current = current.getParent()) { + String known = coordinateByModuleDir.get(current); + if (known != null) { + return Optional.of(current); + } + Optional coordinate = ModuleCoordinates.nameOf(current); + if (coordinate.isPresent()) { + coordinateByModuleDir.put(current, coordinate.get()); + return Optional.of(current); + } + if (current.equals(root)) { + break; + } + } + return Optional.empty(); + } + + /** + * The id path for {@code file} — what follows the language segment of its {@code can://} id. + * Prefixed with the module coordinate when one applies, otherwise the bare + * {@code --input}-relative path. + */ + public String idPath(Path file) { + Path absolute = file.toAbsolutePath().normalize(); + Path moduleDir = moduleDirByFile.get(absolute); + if (moduleDir == null) { + return posix(inputRoot.relativize(absolute)); + } + return coordinateByModuleDir.get(moduleDir) + "/" + posix(moduleDir.relativize(absolute)); + } + + /** + * Coordinates that were claimed by several module directories and therefore applied to none. + * The caller warns on these; an empty map is the ordinary case. + */ + public Map> contested() { + return contested; + } + + private static String posix(Path path) { + return path.toString().replace('\\', '/'); + } +} diff --git a/src/test/java/com/ibm/cldk/L3WalaOverlaysTest.java b/src/test/java/com/ibm/cldk/L3WalaOverlaysTest.java deleted file mode 100644 index f3f78f53..00000000 --- a/src/test/java/com/ibm/cldk/L3WalaOverlaysTest.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.ibm.cldk; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import com.ibm.cldk.schema.JModule; -import java.lang.reflect.Method; -import java.util.LinkedHashMap; -import java.util.Map; -import org.junit.jupiter.api.Test; - -public class L3WalaOverlaysTest { - - @SuppressWarnings("unchecked") - private static String invokeDeriveApplicationId(Map modules) throws Exception { - Method method = L3WalaOverlays.class.getDeclaredMethod("deriveApplicationId", Map.class); - method.setAccessible(true); - return (String) method.invoke(null, modules); - } - - @Test - public void deriveApplicationIdFallsBackToCanUnknownWhenTheModuleHasNoId() throws Exception { - // A module map from which no applicationId can be derived (its one module's id is unset) - // must fall back to "can://unknown" -- not "can:///unknown" (the old SCHEME + "/" + "unknown" - // spelling, back when SCHEME was "can://java") and not "can://java/unknown" (the pre-Task-1 - // shape). - Map modules = new LinkedHashMap<>(); - modules.put("src/A.java", new JModule()); - - String applicationId = invokeDeriveApplicationId(modules); - - assertEquals("can://unknown", applicationId); - } -} diff --git a/src/test/java/com/ibm/cldk/artifacts/ModuleCoordinatesTest.java b/src/test/java/com/ibm/cldk/artifacts/ModuleCoordinatesTest.java new file mode 100644 index 00000000..420efb7c --- /dev/null +++ b/src/test/java/com/ibm/cldk/artifacts/ModuleCoordinatesTest.java @@ -0,0 +1,71 @@ +package com.ibm.cldk.artifacts; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class ModuleCoordinatesTest { + + @Test + void readsTheProjectArtifactIdFromAPom(@TempDir Path dir) throws IOException { + Files.writeString(dir.resolve("pom.xml"), + "4.0.0" + + "org.apache.geronimo" + + "daytrader-web-service" + + "1.0"); + assertEquals(Optional.of("daytrader-web-service"), ModuleCoordinates.nameOf(dir)); + } + + @Test + void ignoresAnArtifactIdThatBelongsToTheParentOrADependency(@TempDir Path dir) throws IOException { + // The module's own coordinate is the one directly under ; a parent's or a + // dependency's spelling names something else entirely and must not be mistaken for it. + Files.writeString(dir.resolve("pom.xml"), + "4.0.0" + + "gthe-parent1" + + "" + + "ga-dependency" + + ""); + assertTrue(ModuleCoordinates.nameOf(dir).isEmpty()); + } + + @Test + void readsRootProjectNameFromSettingsGradle(@TempDir Path dir) throws IOException { + Files.writeString(dir.resolve("settings.gradle"), "rootProject.name = 'daytrader-account'\n"); + assertEquals(Optional.of("daytrader-account"), ModuleCoordinates.nameOf(dir)); + } + + @Test + void aGradleModuleWithNoDeclaredNameHasNoCoordinate(@TempDir Path dir) throws IOException { + // Deliberately NOT the directory name: falling back to the path would reintroduce exactly + // the location-dependence a declared coordinate exists to avoid. + Files.writeString(dir.resolve("build.gradle"), "plugins { id 'java' }\n"); + assertTrue(ModuleCoordinates.nameOf(dir).isEmpty()); + } + + @Test + void aDirectoryWithNoManifestHasNoCoordinate(@TempDir Path dir) { + assertTrue(ModuleCoordinates.nameOf(dir).isEmpty()); + } + + @Test + void aMalformedPomIsEmptyRatherThanThrowing(@TempDir Path dir) throws IOException { + Files.writeString(dir.resolve("pom.xml"), "unclosed"); + assertTrue(ModuleCoordinates.nameOf(dir).isEmpty()); + } + + @Test + void aPomDeclaringADoctypeIsRefused(@TempDir Path dir) throws IOException { + // Manifests keep the categorical DOCTYPE refusal that ManifestParsers documents. + Files.writeString(dir.resolve("pom.xml"), + "]>\n" + + "sneaky"); + assertTrue(ModuleCoordinates.nameOf(dir).isEmpty()); + } +} diff --git a/src/test/java/com/ibm/cldk/schema/L2CallGraphGateTest.java b/src/test/java/com/ibm/cldk/schema/L2CallGraphGateTest.java index 4c7e12e7..ea47750a 100644 --- a/src/test/java/com/ibm/cldk/schema/L2CallGraphGateTest.java +++ b/src/test/java/com/ibm/cldk/schema/L2CallGraphGateTest.java @@ -45,14 +45,28 @@ class L2CallGraphGateTest { private static final ObjectMapper MAPPER = new ObjectMapper(); private static final String APP_ID = CanId.applicationId(APP); + + /** + * The fixture's declared module coordinate — {@code rootProject.name} in its settings.gradle — + * which leads the id path. The {@code symbol_table} key stays the plain relative path, so the + * two deliberately differ and neither can be derived from the other. + * + *

Here the coordinate and the app name coincide, so the id reads + * {@code can://call-graph-test/java/call-graph-test/...}. That is the documented redundancy for + * a single-module project analysed at its own root, not a defect: suppressing the segment when + * it matches the app name would make ids take two shapes depending on a name coincidence. + */ + private static final String MODULE = "call-graph-test/"; + private static final String USER = CanId.childId( - CanId.moduleId(APP_ID, "src/main/java/org/example/User.java"), "User"); + CanId.moduleId(APP_ID, MODULE + "src/main/java/org/example/User.java"), "User"); private static final String HELLO = USER + "/helloString()"; private static final String LOG = USER + "/log()"; private static final String GETNAME = USER + "/getName()"; private static final String LOGLOG = USER + "/loglog()"; private static final String GREETER = CanId.childId( - CanId.moduleId(APP_ID, "src/main/java/org/example/greeting/Greeter.java"), "Greeter"); + CanId.moduleId(APP_ID, MODULE + "src/main/java/org/example/greeting/Greeter.java"), + "Greeter"); private static final String GREET = GREETER + "/greet(java.lang.String)"; private static final String TRIM = CanId.externalId(APP, "java.lang.String", "trim()"); diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/L1BuildContextTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/L1BuildContextTest.java new file mode 100644 index 00000000..28e2cb39 --- /dev/null +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/L1BuildContextTest.java @@ -0,0 +1,41 @@ +package com.ibm.cldk.syntactic_analysis; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.ibm.cldk.schema.CanId; +import org.junit.jupiter.api.Test; + +class L1BuildContextTest { + + private static final String APP = CanId.applicationId("daytrader"); + + @Test + void theModuleIdIsBuiltFromTheIdPathAndNotFromTheFileKey() { + // The two differ whenever a module declares a coordinate: the key stays the real + // --input-relative path (uniqueness is the key's job), while the id carries the coordinate. + L1BuildContext ctx = new L1BuildContext( + APP, + "src/main/java/com/foo/Bar.java", + "daytrader-web-service/src/main/java/com/foo/Bar.java", + "class Bar {}", + 1, + 3, + "ast", + null); + + assertEquals( + "can://daytrader/java/daytrader-web-service/src/main/java/com/foo/Bar.java", + ctx.moduleId()); + assertEquals("src/main/java/com/foo/Bar.java", ctx.getFileKey()); + } + + @Test + void aContextBuiltWithoutAnIdPathFallsBackToTheFileKey() { + // Every convenience constructor defaults one to the other, so a module with no resolvable + // coordinate keeps exactly the id it had before coordinates existed. + L1BuildContext ctx = + new L1BuildContext(APP, "src/main/java/com/foo/Bar.java", "class Bar {}"); + + assertEquals("can://daytrader/java/src/main/java/com/foo/Bar.java", ctx.moduleId()); + } +} diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/ModulePrefixesTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/ModulePrefixesTest.java new file mode 100644 index 00000000..396aa4e1 --- /dev/null +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/ModulePrefixesTest.java @@ -0,0 +1,112 @@ +package com.ibm.cldk.syntactic_analysis; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class ModulePrefixesTest { + + private static Path pom(Path dir, String artifactId) throws IOException { + Files.createDirectories(dir); + Files.writeString(dir.resolve("pom.xml"), + "4.0.0g" + + "" + artifactId + "1"); + return dir; + } + + private static Path java(Path moduleDir, String rel) throws IOException { + Path file = moduleDir.resolve(rel); + Files.createDirectories(file.getParent()); + Files.writeString(file, "class X {}"); + return file; + } + + @Test + void theInputItselfIsAModuleAndItsCoordinateLeadsTheIdPath(@TempDir Path dir) throws IOException { + pom(dir, "daytrader-web-service"); + Path file = java(dir, "src/main/java/com/foo/Bar.java"); + + ModulePrefixes prefixes = ModulePrefixes.resolve(dir, List.of(file)); + + assertEquals("daytrader-web-service/src/main/java/com/foo/Bar.java", prefixes.idPath(file)); + } + + @Test + void aReactorChildCarriesItsOwnCoordinateNotTheAggregatorsAndNotItsDirectoryName(@TempDir Path dir) + throws IOException { + pom(dir, "daytrader-parent"); + Path child = pom(dir.resolve("web"), "daytrader-web-service"); + Path file = java(child, "src/main/java/com/foo/Bar.java"); + + ModulePrefixes prefixes = ModulePrefixes.resolve(dir, List.of(file)); + + // Nearest enclosing manifest wins: the child's coordinate, and the path below it is + // relative to the child, not to the input. + assertEquals("daytrader-web-service/src/main/java/com/foo/Bar.java", prefixes.idPath(file)); + } + + @Test + void noManifestAnywhereLeavesTheInputRelativePathUnprefixed(@TempDir Path dir) throws IOException { + Path file = java(dir, "src/main/java/com/foo/Bar.java"); + + ModulePrefixes prefixes = ModulePrefixes.resolve(dir, List.of(file)); + + assertEquals("src/main/java/com/foo/Bar.java", prefixes.idPath(file)); + } + + @Test + void theSearchIsBoundedAtTheInputSoAManifestAboveItIsInvisible(@TempDir Path dir) throws IOException { + pom(dir, "the-enclosing-repo"); + Path input = Files.createDirectories(dir.resolve("service")); + Path file = java(input, "src/main/java/com/foo/Bar.java"); + + ModulePrefixes prefixes = ModulePrefixes.resolve(input, List.of(file)); + + assertEquals("src/main/java/com/foo/Bar.java", prefixes.idPath(file)); + } + + @Test + void aCoordinateClaimedByTwoModuleDirectoriesIsAppliedToNeither(@TempDir Path dir) throws IOException { + // Vendored duplicates: several services each build one shared internal library under the + // same module name. Applying the prefix would collide their ids, so it applies to neither. + Path a = pom(dir.resolve("svc-a/lib"), "daytrader-core"); + Path b = pom(dir.resolve("svc-b/lib"), "daytrader-core"); + Path fileA = java(a, "src/main/java/com/shared/Bean.java"); + Path fileB = java(b, "src/main/java/com/shared/Bean.java"); + + ModulePrefixes prefixes = ModulePrefixes.resolve(dir, List.of(fileA, fileB)); + + assertEquals("svc-a/lib/src/main/java/com/shared/Bean.java", prefixes.idPath(fileA)); + assertEquals("svc-b/lib/src/main/java/com/shared/Bean.java", prefixes.idPath(fileB)); + assertEquals(List.of("daytrader-core"), List.copyOf(prefixes.contested().keySet())); + assertEquals(2, prefixes.contested().get("daytrader-core").size()); + } + + @Test + void oneModuleDirectoryClaimingACoordinateTwiceIsNotAConflict(@TempDir Path dir) throws IOException { + // Two files of the SAME module resolve to one manifest directory; that is not a collision. + pom(dir, "daytrader-web-service"); + Path one = java(dir, "src/main/java/com/foo/Bar.java"); + Path two = java(dir, "src/main/java/com/foo/Baz.java"); + + ModulePrefixes prefixes = ModulePrefixes.resolve(dir, List.of(one, two)); + + assertEquals("daytrader-web-service/src/main/java/com/foo/Bar.java", prefixes.idPath(one)); + assertEquals("daytrader-web-service/src/main/java/com/foo/Baz.java", prefixes.idPath(two)); + assertTrue(prefixes.contested().isEmpty()); + } + + @Test + void separatorsAreNormalizedToForwardSlashes(@TempDir Path dir) throws IOException { + pom(dir, "svc"); + Path file = java(dir, "src/main/java/com/foo/Bar.java"); + + assertTrue(ModulePrefixes.resolve(dir, List.of(file)).idPath(file).indexOf('\\') < 0); + } +}