Skip to content

Commit 32eb185

Browse files
committed
fix(gui): correct java pluginId mapping and reduce saveAllChanges NPath
Three issues surfaced by the GUI module build: 1. SourceWriteBackSupportTest failures (findPatcher_javaFile_returnsJvmPatcher, supportedLanguagesLabel_listsJavaAndCSharp) SourceWriteBackSupport.LANGUAGE_LABELS used the key "jvm" but JavaSourcePatcher.pluginId() actually returns "java". The getOrDefault fallback uppercased the unmapped key, so supportedLanguagesLabel() returned "JAVA, C#" instead of "Java, C#" and the case-sensitive contains("Java") assertion failed. Fix: change the key to "java" and document the contract (LANGUAGE_LABELS keys must match the patchers' pluginId() return values). The test that asserted patcher.pluginId() == "jvm" is renamed to findPatcher_javaFile_returnsJavaPatcher and switched to assertEquals so the actual vs. expected value is reported on failure. 2. MainWindow.saveAllChanges NPath 289 (threshold 260) Extracted the body into five single-responsibility helpers: - groupStagedByFile — bucket entries per source file - ensureWriteBackSupport — lazy SourceWriteBackSupport init - patchSingleFile — per-file SourcePatcher invocation + audit-entry build-up - writeAuditEvidence — .methodatlas/ audit log + warning dialog on failure - reportSaveResult — status-bar message / error dialog saveAllChanges itself is now a short orchestrator. Semantics are unchanged; the public behaviour (which files get patched, which audit entries are written, which dialogs are shown) is bit-identical. 3. UnnecessaryWarningSuppression on SourceWriteBackSupport constructor and on patchSingleFile Removed @SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops") from both — PMD does not actually emit the suppressed finding for these methods, so the annotation was flagged as unnecessary. The suppression on groupStagedByFile is kept because PMD does flag the `new ArrayList<>()` lambda inside computeIfAbsent there. Confirmed by the user: PMD passes and all tests pass.
1 parent 6a9379b commit 32eb185

3 files changed

Lines changed: 149 additions & 79 deletions

File tree

methodatlas-gui/src/main/java/org/egothor/methodatlas/gui/MainWindow.java

Lines changed: 135 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -319,102 +319,164 @@ private void refreshProfileCombo() {
319319

320320
// ── Save All Changes ──────────────────────────────────────────────────
321321

322-
@SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops")
322+
/**
323+
* Persists every staged {@link MethodEntry} to disk by routing each
324+
* source file through its matching {@link SourcePatcher}. Files in a
325+
* language with no patcher (TypeScript, Go, Python, …) are collected
326+
* as per-file errors so the user gets an explicit message instead of
327+
* a silent skip.
328+
*
329+
* <p>The actual work is delegated to small single-responsibility
330+
* helpers ({@link #groupStagedByFile}, {@link #ensureWriteBackSupport},
331+
* {@link #patchSingleFile}, {@link #writeAuditEvidence},
332+
* {@link #reportSaveResult}) to keep the method's NPath complexity
333+
* within the project PMD threshold.</p>
334+
*/
323335
private void saveAllChanges() {
324336
List<MethodEntry> staged = model.getStagedEntries();
325-
if (staged.isEmpty()) { return; }
326-
327-
// Group staged entries by source file
328-
Map<Path, List<MethodEntry>> byFile = new LinkedHashMap<>();
329-
for (MethodEntry entry : staged) {
330-
Path fp = entry.discovered().filePath();
331-
if (fp != null) { byFile.computeIfAbsent(fp, k -> new ArrayList<>()).add(entry); }
337+
if (staged.isEmpty()) {
338+
return;
332339
}
333340

334-
// Reuse (or lazily build) the SourceWriteBackSupport that gates the
335-
// tag editor. This guarantees the same configured patchers are used
336-
// for both UI gating and on-disk write-back.
337-
if (writeBackSupport == null) {
338-
TestDiscoveryConfig config = new TestDiscoveryConfig(
339-
TagEditorPanel.buildFlatSuffixes(settings),
340-
Set.copyOf(settings.getTestAnnotations()),
341-
Map.of());
342-
writeBackSupport = new SourceWriteBackSupport(config);
343-
tagEditorPanel.setWriteBackSupport(writeBackSupport);
344-
}
341+
Map<Path, List<MethodEntry>> byFile = groupStagedByFile(staged);
342+
ensureWriteBackSupport();
345343

346344
List<String> errors = new ArrayList<>();
347345
Set<Path> savedFiles = new LinkedHashSet<>();
348346
List<AuditWriter.SavedEntry> auditEntries = new ArrayList<>();
349347

350348
for (Map.Entry<Path, List<MethodEntry>> fe : byFile.entrySet()) {
351-
Path filePath = fe.getKey();
352-
List<MethodEntry> entries = fe.getValue();
353-
354-
SourcePatcher patcher = writeBackSupport.findPatcher(filePath);
355-
if (patcher == null) {
356-
errors.add(filePath.getFileName()
357-
+ ": source write-back is not supported for this language "
358-
+ "(supported: " + writeBackSupport.supportedLanguagesLabel() + ")");
359-
continue;
360-
}
349+
patchSingleFile(fe.getKey(), fe.getValue(), errors, savedFiles, auditEntries);
350+
}
361351

362-
Map<String, List<String>> tagsToApply = new LinkedHashMap<>();
363-
Map<String, String> displayNames = new LinkedHashMap<>();
364-
for (MethodEntry e : entries) {
365-
tagsToApply.put(e.discovered().method(), e.getPendingTags());
366-
String dn = e.getPendingDisplayName();
367-
if (dn != null) { displayNames.put(e.discovered().method(), dn); }
368-
}
352+
editorPanel.reloadIfAmong(savedFiles);
353+
saveAllButton.setEnabled(model.hasStagedChanges());
369354

370-
StringWriter sw = new StringWriter();
371-
try {
372-
patcher.patch(filePath, tagsToApply, displayNames, new PrintWriter(sw));
373-
for (MethodEntry e : entries) {
374-
// Snapshot before clearing so AuditWriter sees the applied values
375-
auditEntries.add(new AuditWriter.SavedEntry(
376-
e.discovered().fqcn(),
377-
e.discovered().method(),
378-
e.discovered().loc(),
379-
e.getPendingTags(),
380-
e.getPendingDisplayName(),
381-
e.suggestion()));
382-
e.setAppliedTags(e.getPendingTags());
383-
e.clearStagedPatch();
384-
model.notifyEntryChanged(e);
385-
}
386-
savedFiles.add(filePath);
387-
} catch (IOException ex) {
388-
errors.add(filePath.getFileName() + ": " + ex.getMessage());
355+
writeAuditEvidence(auditEntries);
356+
reportSaveResult(staged, savedFiles, errors);
357+
}
358+
359+
/**
360+
* Groups every staged entry by its source-file path. Entries whose
361+
* {@code filePath()} is {@code null} (e.g. unsaved buffers) are
362+
* silently skipped because they cannot be written to disk.
363+
*/
364+
@SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops")
365+
private static Map<Path, List<MethodEntry>> groupStagedByFile(List<MethodEntry> staged) {
366+
Map<Path, List<MethodEntry>> byFile = new LinkedHashMap<>();
367+
for (MethodEntry entry : staged) {
368+
Path fp = entry.discovered().filePath();
369+
if (fp != null) {
370+
byFile.computeIfAbsent(fp, k -> new ArrayList<>()).add(entry);
389371
}
390372
}
373+
return byFile;
374+
}
391375

392-
editorPanel.reloadIfAmong(savedFiles);
393-
saveAllButton.setEnabled(model.hasStagedChanges());
376+
/**
377+
* Lazily creates the shared {@link SourceWriteBackSupport} and hands
378+
* it to the tag editor so UI gating and on-disk write-back agree on
379+
* which languages are supported.
380+
*/
381+
private void ensureWriteBackSupport() {
382+
if (writeBackSupport != null) {
383+
return;
384+
}
385+
TestDiscoveryConfig config = new TestDiscoveryConfig(
386+
TagEditorPanel.buildFlatSuffixes(settings),
387+
Set.copyOf(settings.getTestAnnotations()),
388+
Map.of());
389+
writeBackSupport = new SourceWriteBackSupport(config);
390+
tagEditorPanel.setWriteBackSupport(writeBackSupport);
391+
}
392+
393+
/**
394+
* Writes all staged changes for one source file via its
395+
* {@link SourcePatcher}. Records per-file errors in {@code errors},
396+
* appends a {@link AuditWriter.SavedEntry} for each persisted method,
397+
* and adds the file to {@code savedFiles} on success.
398+
*/
399+
private void patchSingleFile(Path filePath, List<MethodEntry> entries,
400+
List<String> errors, Set<Path> savedFiles,
401+
List<AuditWriter.SavedEntry> auditEntries) {
402+
SourcePatcher patcher = writeBackSupport.findPatcher(filePath);
403+
if (patcher == null) {
404+
errors.add(filePath.getFileName()
405+
+ ": source write-back is not supported for this language "
406+
+ "(supported: " + writeBackSupport.supportedLanguagesLabel() + ")");
407+
return;
408+
}
394409

395-
// Write audit evidence — warn on failure, do not roll back source patches
396-
if (!auditEntries.isEmpty()) {
397-
String dir = dirField.getText().trim();
398-
if (!dir.isEmpty()) {
399-
try {
400-
AuditWriter.write(Path.of(dir), auditEntries, settings.getOperatorName());
401-
} catch (IOException ex) {
402-
JOptionPane.showMessageDialog(this,
403-
"Source files were saved but audit records could not be written to .methodatlas/:\n"
404-
+ ex.getMessage(),
405-
"Audit Write Warning", JOptionPane.WARNING_MESSAGE);
406-
}
410+
Map<String, List<String>> tagsToApply = new LinkedHashMap<>();
411+
Map<String, String> displayNames = new LinkedHashMap<>();
412+
for (MethodEntry e : entries) {
413+
tagsToApply.put(e.discovered().method(), e.getPendingTags());
414+
String dn = e.getPendingDisplayName();
415+
if (dn != null) {
416+
displayNames.put(e.discovered().method(), dn);
407417
}
408418
}
409419

420+
StringWriter sw = new StringWriter();
421+
try {
422+
patcher.patch(filePath, tagsToApply, displayNames, new PrintWriter(sw));
423+
for (MethodEntry e : entries) {
424+
// Snapshot before clearing so AuditWriter sees the applied values.
425+
auditEntries.add(new AuditWriter.SavedEntry(
426+
e.discovered().fqcn(),
427+
e.discovered().method(),
428+
e.discovered().loc(),
429+
e.getPendingTags(),
430+
e.getPendingDisplayName(),
431+
e.suggestion()));
432+
e.setAppliedTags(e.getPendingTags());
433+
e.clearStagedPatch();
434+
model.notifyEntryChanged(e);
435+
}
436+
savedFiles.add(filePath);
437+
} catch (IOException ex) {
438+
errors.add(filePath.getFileName() + ": " + ex.getMessage());
439+
}
440+
}
441+
442+
/**
443+
* Writes the audit log to the project's {@code .methodatlas/} folder
444+
* (rooted at the currently scanned directory). Audit-write failures
445+
* are surfaced as a warning dialog but do <strong>not</strong> roll
446+
* back the source-file patches that have already been written.
447+
*/
448+
private void writeAuditEvidence(List<AuditWriter.SavedEntry> auditEntries) {
449+
if (auditEntries.isEmpty()) {
450+
return;
451+
}
452+
String dir = dirField.getText().trim();
453+
if (dir.isEmpty()) {
454+
return;
455+
}
456+
try {
457+
AuditWriter.write(Path.of(dir), auditEntries, settings.getOperatorName());
458+
} catch (IOException ex) {
459+
JOptionPane.showMessageDialog(this,
460+
"Source files were saved but audit records could not be written to .methodatlas/:\n"
461+
+ ex.getMessage(),
462+
"Audit Write Warning", JOptionPane.WARNING_MESSAGE);
463+
}
464+
}
465+
466+
/**
467+
* Sets the status-bar message on a clean save, or shows an error
468+
* dialog listing every per-file failure when any error occurred.
469+
*/
470+
private void reportSaveResult(List<MethodEntry> staged,
471+
Set<Path> savedFiles, List<String> errors) {
410472
if (errors.isEmpty()) {
411473
model.setStatusMessage("Saved " + staged.size() + " method(s) across "
412474
+ savedFiles.size() + " file(s)");
413-
} else {
414-
JOptionPane.showMessageDialog(this,
415-
"Some files could not be saved:\n" + String.join("\n", errors),
416-
"Save Error", JOptionPane.ERROR_MESSAGE);
475+
return;
417476
}
477+
JOptionPane.showMessageDialog(this,
478+
"Some files could not be saved:\n" + String.join("\n", errors),
479+
"Save Error", JOptionPane.ERROR_MESSAGE);
418480
}
419481

420482
// ── Model observer ────────────────────────────────────────────────────

methodatlas-gui/src/main/java/org/egothor/methodatlas/gui/service/SourceWriteBackSupport.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,16 @@
3737
*/
3838
public final class SourceWriteBackSupport {
3939

40+
/**
41+
* Mapping from a patcher's {@code pluginId()} to the human-readable
42+
* language label used in {@link #supportedLanguagesLabel()}. Keys must
43+
* match the {@code pluginId()} return values of the actual
44+
* {@link SourcePatcher} implementations:
45+
* {@code JavaSourcePatcher.pluginId() == "java"} and
46+
* {@code DotNetSourcePatcher.pluginId() == "dotnet"}.
47+
*/
4048
private static final Map<String, String> LANGUAGE_LABELS = Map.of(
41-
"jvm", "Java",
49+
"java", "Java",
4250
"dotnet", "C#"
4351
);
4452

@@ -51,7 +59,6 @@ public final class SourceWriteBackSupport {
5159
* @param config runtime configuration forwarded to every patcher; never
5260
* {@code null}
5361
*/
54-
@SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops")
5562
public SourceWriteBackSupport(TestDiscoveryConfig config) {
5663
List<SourcePatcher> loaded = new ArrayList<>();
5764
ServiceLoader.load(SourcePatcher.class).forEach(p -> {

methodatlas-gui/src/test/java/org/egothor/methodatlas/gui/service/SourceWriteBackSupportTest.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.egothor.methodatlas.gui.service;
22

3+
import static org.junit.jupiter.api.Assertions.assertEquals;
34
import static org.junit.jupiter.api.Assertions.assertFalse;
45
import static org.junit.jupiter.api.Assertions.assertNotNull;
56
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -98,12 +99,12 @@ void supports_nullPath_returnsFalse() {
9899
}
99100

100101
@Test
101-
void findPatcher_javaFile_returnsJvmPatcher() {
102+
void findPatcher_javaFile_returnsJavaPatcher() {
102103
SourceWriteBackSupport sut = new SourceWriteBackSupport(DEFAULT_CONFIG);
103104
SourcePatcher patcher = sut.findPatcher(Path.of("FooTest.java"));
104-
assertNotNull(patcher, "expected JVM patcher for .java file");
105-
assertTrue(patcher.pluginId().equals("jvm"),
106-
"expected pluginId() == jvm, got " + patcher.pluginId());
105+
assertNotNull(patcher, "expected Java patcher for .java file");
106+
assertEquals("java", patcher.pluginId(),
107+
"expected pluginId() == java, got " + patcher.pluginId());
107108
}
108109

109110
@Test

0 commit comments

Comments
 (0)