feat(#4983): enable WPA caching in MjLint #4986
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,6 +207,40 @@ private int lintAll(final Map<Severity, Integer> counts) throws IOException { | |
| if (!this.skipProgramLints.isEmpty()) { | ||
| Logger.info(this, "Unliting WPA lints: %[list]s", this.skipProgramLints); | ||
| } | ||
| final List<Defect> defects; | ||
| if (this.cacheEnabled) { | ||
| final Path wpa = Path.of("wpa.xmir"); | ||
| final Path target = this.targetDir.toPath().resolve(MjLint.DIR).resolve(wpa); | ||
| new Cache( | ||
| this.cache.toPath().resolve(MjLint.CACHE), | ||
| root -> { | ||
| Logger.info(this, "Linting a package"); | ||
| final Directives all = new Directives().add("defects"); | ||
| for (final Defect defect : this.wpa(pkg)) { | ||
| MjLint.embedded(all, defect); | ||
| } | ||
| all.up(); | ||
| return new Xembler(all).xmlQuietly(); | ||
| }, | ||
| p -> p.getFileName().toString().endsWith(".xmir") | ||
| && !p.getFileName().equals(wpa) | ||
| ).apply(this.sourcesDir.toPath(), target, wpa); | ||
|
Comment on lines
+212
to
+227
There was a problem hiding this comment. In the WPA cached branch, the cache key is computed from Consider hashing the actual WPA inputs (e.g., the XMIR directory under |
||
| defects = MjLint.read(target); | ||
| } else { | ||
| Logger.info( | ||
| this, | ||
| "Linting a package without cache, this might be slow, consider enabling cache" | ||
| ); | ||
| defects = this.wpa(pkg); | ||
| } | ||
| for (final Defect defect : defects) { | ||
| counts.compute(defect.severity(), (sev, before) -> before + 1); | ||
| } | ||
|
Comment on lines
+210
to
+238
There was a problem hiding this comment. With cache enabled, WPA defects are only logged during a cache miss (inside the compilation lambda via If consistent logging is desired, log defects after loading them from cache as well. That likely requires caching/restoring the full defect details (object/line/text) instead of dropping them in |
||
| return pkg.size(); | ||
| } | ||
|
|
||
| private List<Defect> wpa(final Map<String, XML> pkg) { | ||
| final List<Defect> defects = new ArrayList<>(0); | ||
| new Program(pkg) | ||
| .without(this.skipProgramLints.toArray(new String[0])) | ||
| .defects() | ||
|
|
@@ -222,12 +256,12 @@ private int lintAll(final Map<Severity, Integer> counts) throws IOException { | |
| ) | ||
| ).applyQuietly(node); | ||
| if (MjLint.notSuppressed(new Xnav(node), defect)) { | ||
| defects.add(defect); | ||
| MjLint.logOne(defect); | ||
| } | ||
| } | ||
| ); | ||
| return defects; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -408,6 +442,23 @@ private static Directives embedded(final Directives dirs, final Defect defect) { | |
| return dirs.up(); | ||
| } | ||
|
|
||
| /** | ||
| * Read defects from XMIR. | ||
| * @param path Path to XMIR | ||
| * @return Collection of defects | ||
| */ | ||
| private static List<Defect> read(final Path path) { | ||
| return new Xnav(path).path("/defects/error").map( | ||
| node -> new Defect.Default( | ||
| node.attribute("check").text().orElseThrow(), | ||
| Severity.parsed(node.attribute("severity").text().orElseThrow()), | ||
| "", | ||
| 0, | ||
| "" | ||
| ) | ||
| ).collect(Collectors.toList()); | ||
|
Comment on lines
+450
to
+459
There was a problem hiding this comment.
It would be more diagnosable to throw with an explicit message here too (e.g., "Cached WPA defect must contain 'check'/'severity' attribute"). |
||
| } | ||
|
Comment on lines
+445
to
+460
Contributor
There was a problem hiding this comment. Defect data is discarded when reading from cache. The Consider extracting available data to preserve full defect information: Proposed fix to preserve defect data private static List<Defect> read(final Path path) {
return new Xnav(path).path("/defects/error").map(
node -> new Defect.Default(
node.attribute("check").text().orElseThrow(),
Severity.parsed(node.attribute("severity").text().orElseThrow()),
"",
- 0,
- ""
+ Integer.parseInt(node.attribute("line").text().orElse("0")),
+ node.text().orElse("")
)
).collect(Collectors.toList());
}🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * This defect is not suppressed? | ||
| * @param xnav The XMIR as {@link Xnav} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new
Cachefeature was added (directory hashing filter viaPredicate<Path>), but there’s no unit test coverage to confirm that excluded files don’t affect the computed hash and that included files do. Since this is now relied on by WPA caching, a focused test would help prevent regressions (e.g., create a directory with two files, exclude one via filter, modify it, and assert the cache does not recompile).