Skip to content

Commit 6029b05

Browse files
gtoisonhazendaz
gtoison
and
hazendaz
authored
fix: revert PR #2894 (#3038)
Keeping track of the files used by bug reporters (to prevent writing the same file multiple times) seems to prevent the html stylesheets to apply, as well as breaking xml:withMessages Revert the changes from #2894 as discussed in #2969 Co-authored-by: Jeremy Landis <jeremylandis@hotmail.com>
1 parent abc4990 commit 6029b05

File tree

3 files changed

+50
-113
lines changed

3 files changed

+50
-113
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
1313
- Check the actual caught exceptions (instead of the their common type) when analyzing multi-catch blocks ([#2968](https://github.com/spotbugs/spotbugs/issues/2968))
1414
- System property `findbugs.refcomp.reportAll` is now being used. For some new conditions, it will emit an experimental warning ([#2988](https://github.com/spotbugs/spotbugs/pull/2988))
1515
- `-version` flag prints the version to the standard output ([#2797](https://github.com/spotbugs/spotbugs/issues/2797))
16+
- Revert the changes from ([#2894](https://github.com/spotbugs/spotbugs/pull/2894)) to get HTML stylesheets to work again ([#2969](https://github.com/spotbugs/spotbugs/issues/2969))
1617

1718
## 4.8.6 - 2024-06-17
1819
### Fixed

‎spotbugs-tests/src/test/java/edu/umd/cs/findbugs/TextUICommandLineTest.java

+8-37
Original file line numberDiff line numberDiff line change
@@ -11,51 +11,24 @@
1111
import static org.hamcrest.Matchers.is;
1212
import static org.hamcrest.MatcherAssert.assertThat;
1313
import static org.junit.jupiter.api.Assertions.assertTrue;
14-
import static org.junit.jupiter.api.Assertions.assertNull;
15-
import static org.junit.jupiter.api.Assertions.assertNotNull;
1614

1715
class TextUICommandLineTest {
1816
@Test
19-
void duplicateFilepathIsIgnored() throws IOException {
17+
void handleOutputFileReturnsRemainingPart() throws IOException {
2018
Path file = Files.createTempFile("spotbugs", ".xml");
2119
TextUICommandLine commandLine = new TextUICommandLine();
20+
SortingBugReporter reporter = new SortingBugReporter();
21+
String result = commandLine.handleOutputFilePath(reporter, "withMessages=" + file.toFile().getAbsolutePath());
2222

23-
TextUIBugReporter call1 = commandLine.initializeReporter("withMessages=" + file.toFile().getAbsolutePath(),
24-
() -> new SortingBugReporter(), (reporter, reporterOptions) -> {
25-
});
26-
TextUIBugReporter call2 = commandLine.initializeReporter("withMessages=" + file.toFile().getAbsolutePath(),
27-
() -> new SortingBugReporter(), (reporter, reporterOptions) -> {
28-
});
29-
30-
assertNotNull(call1);
31-
assertNull(call2);
32-
}
33-
34-
@Test
35-
void differentFilepathsOnSameReporter_createsTwoReporters() throws IOException {
36-
Path file = Files.createTempFile("spotbugs", ".xml");
37-
Path file2 = Files.createTempFile("spotbugs", ".xml");
38-
TextUICommandLine commandLine = new TextUICommandLine();
39-
40-
TextUIBugReporter call1 = commandLine.initializeReporter("withMessages=" + file.toFile().getAbsolutePath(),
41-
() -> new SortingBugReporter(), (reporter, reporterOptions) -> {
42-
});
43-
TextUIBugReporter call2 = commandLine.initializeReporter("withMessages=" + file2.toFile().getAbsolutePath(),
44-
() -> new SortingBugReporter(), (reporter, reporterOptions) -> {
45-
});
46-
47-
assertNotNull(call1);
48-
assertNotNull(call2);
23+
assertThat(result, is("withMessages"));
4924
}
5025

5126
@Test
5227
void handleOutputFilePathUsesGzip() throws IOException {
5328
Path file = Files.createTempFile("spotbugs", ".xml.gz");
5429
TextUICommandLine commandLine = new TextUICommandLine();
55-
TextUIBugReporter reporter = commandLine.initializeReporter("withMessages=" + file.toFile().getAbsolutePath(),
56-
() -> new SortingBugReporter(),
57-
(constructed, reporterOptions) -> {
58-
});
30+
SortingBugReporter reporter = new SortingBugReporter();
31+
commandLine.handleOutputFilePath(reporter, "withMessages=" + file.toFile().getAbsolutePath());
5932

6033
reporter.finish();
6134
byte[] written = Files.readAllBytes(file);
@@ -68,10 +41,8 @@ void handleOutputFileTruncatesExisting() throws IOException {
6841
Path file = Files.createTempFile("spotbugs", ".html");
6942
Files.writeString(file, "content");
7043
TextUICommandLine commandLine = new TextUICommandLine();
71-
TextUIBugReporter reporter = commandLine.initializeReporter("withMessages=" + file.toFile().getAbsolutePath(),
72-
() -> new SortingBugReporter(),
73-
(constructed, reporterOptions) -> {
74-
});
44+
SortingBugReporter reporter = new SortingBugReporter();
45+
commandLine.handleOutputFilePath(reporter, "withMessages=" + file.toFile().getAbsolutePath());
7546

7647
reporter.finish();
7748
byte[] written = Files.readAllBytes(file);

‎spotbugs/src/main/java/edu/umd/cs/findbugs/TextUICommandLine.java

+41-76
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040
import java.util.Map;
4141
import java.util.Set;
4242
import java.util.StringTokenizer;
43-
import java.util.function.BiConsumer;
44-
import java.util.function.Supplier;
4543
import java.util.zip.GZIPOutputStream;
4644

4745
import javax.annotation.CheckForNull;
@@ -117,8 +115,6 @@ private interface Chooser {
117115

118116
private final Set<String> disabledBugReporterDecorators = new LinkedHashSet<>();
119117

120-
private final Set<Path> usedReporterPaths = new LinkedHashSet<>();
121-
122118
private boolean setExitCode = false;
123119

124120
private boolean noClassOk = false;
@@ -270,40 +266,30 @@ public boolean justPrintVersion() {
270266
private List<TextUIBugReporter> reporters = new ArrayList<>();
271267

272268
/**
273-
* Parse {@code optionExtraPart} and create a path-associated {@Link TextUIBugReporter} if it contains the
269+
* Parse {@code optionExtraPart} and configure {@Link TextUIBugReporter} if it contains the
274270
* output file path such as {@code ":withMessages=path/to/file.extension"} and {@code "=/absolute/path/to/file.extension"}.
275-
* Finally configure the created BugReporter with the {@code optionExtraPart}'s configuration information.
276271
*
272+
* @param reporter the reporter to set a {@link PrintStream} based on the given file path
277273
* @param optionExtraPart extra part of the specified commandline option
278-
* @param ctor A supplier for an unconfigured {@Link TextUIBugReporter}.
279-
* @param handleOptions A function that can configure the created {@code BugReporter} instance.
280-
* @param <T> The implementation type of the {@Link TextUIBugReporter} to propagate type
281-
* information between {@link ctor} and {@link handleOptions}.
282-
* @return The fully configured reporter, or {@code null}, if the reporter would output to a file that is already used as a reporter output file.
274+
* @return Remaining part of {@code optionExtraPart}
283275
*/
284-
/* visible for testing */ <T extends TextUIBugReporter> TextUIBugReporter initializeReporter(String optionExtraPart,
285-
Supplier<T> ctor, BiConsumer<T, String> handleOptions) {
276+
/* visible for testing */ String handleOutputFilePath(TextUIBugReporter reporter, String optionExtraPart) {
286277
int index = optionExtraPart.indexOf('=');
287-
T reporter = ctor.get();
288278
if (index >= 0) {
289279
Path path = Paths.get(optionExtraPart.substring(index + 1));
290-
if (this.usedReporterPaths.contains(path)) {
291-
return null;
292-
}
293280
try {
294281
OutputStream oStream = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE,
295282
StandardOpenOption.TRUNCATE_EXISTING);
296283
if ("gz".equals(Util.getFileExtension(path.toFile()))) {
297284
oStream = new GZIPOutputStream(oStream);
298285
}
299286
reporter.setOutputStream(UTF8.printStream(oStream));
300-
usedReporterPaths.add(path);
301287
} catch (IOException e) {
302288
throw new UncheckedIOException(e);
303289
}
304-
handleOptions.accept(reporter, optionExtraPart.substring(0, index));
290+
optionExtraPart = optionExtraPart.substring(0, index);
305291
}
306-
return reporter;
292+
return optionExtraPart;
307293
}
308294

309295
@SuppressFBWarnings("DM_EXIT")
@@ -357,44 +343,35 @@ protected void handleOption(String option, String optionExtraPart) {
357343
mergeSimilarWarnings = false;
358344
} else if ("-sortByClass".equals(option)) {
359345
bugReporterType = SORTING_REPORTER;
360-
TextUIBugReporter reporter = initializeReporter(optionExtraPart,
361-
() -> new SortingBugReporter(),
362-
(r, rOpts) -> {
363-
});
364-
if (reporter != null) {
365-
reporters.add(reporter);
366-
}
346+
SortingBugReporter sortingBugReporter = new SortingBugReporter();
347+
handleOutputFilePath(sortingBugReporter, optionExtraPart);
348+
reporters.add(sortingBugReporter);
367349
} else if ("-xml".equals(option)) {
368350
bugReporterType = XML_REPORTER;
369-
TextUIBugReporter reporter = initializeReporter(
370-
optionExtraPart,
371-
() -> new XMLBugReporter(project),
372-
(r, arg) -> {
373-
if (!"".equals(arg)) {
374-
if ("withMessages".equals(arg)) {
375-
r.setAddMessages(true);
376-
} else if ("withAbridgedMessages".equals(arg)) {
377-
r.setAddMessages(true);
378-
xmlWithAbridgedMessages = true;
379-
} else if ("minimal".equals(arg)) {
380-
r.setMinimalXML(true);
381-
} else {
382-
throw new IllegalArgumentException("Unknown option: -xml:" + arg);
383-
}
384-
}
385-
});
386-
if (reporter != null) {
387-
reporters.add(reporter);
351+
XMLBugReporter xmlBugReporter = new XMLBugReporter(project);
352+
optionExtraPart = handleOutputFilePath(xmlBugReporter, optionExtraPart);
353+
boolean xmlWithMessages = false;
354+
boolean xmlMinimal = false;
355+
if (!"".equals(optionExtraPart)) {
356+
if ("withMessages".equals(optionExtraPart)) {
357+
xmlWithMessages = true;
358+
} else if ("withAbridgedMessages".equals(optionExtraPart)) {
359+
xmlWithMessages = true;
360+
xmlWithAbridgedMessages = true;
361+
} else if ("minimal".equals(optionExtraPart)) {
362+
xmlMinimal = true;
363+
} else {
364+
throw new IllegalArgumentException("Unknown option: -xml:" + optionExtraPart);
365+
}
388366
}
367+
xmlBugReporter.setAddMessages(xmlWithMessages);
368+
xmlBugReporter.setMinimalXML(xmlMinimal);
369+
reporters.add(xmlBugReporter);
389370
} else if ("-emacs".equals(option)) {
371+
EmacsBugReporter emacsBugReporter = new EmacsBugReporter();
372+
handleOutputFilePath(emacsBugReporter, optionExtraPart);
373+
reporters.add(emacsBugReporter);
390374
bugReporterType = EMACS_REPORTER;
391-
TextUIBugReporter reporter = initializeReporter(optionExtraPart,
392-
() -> new EmacsBugReporter(),
393-
(r, rOpts) -> {
394-
});
395-
if (reporter != null) {
396-
reporters.add(reporter);
397-
}
398375
} else if ("-relaxed".equals(option)) {
399376
relaxedReportingMode = true;
400377
} else if ("-train".equals(option)) {
@@ -403,34 +380,22 @@ protected void handleOption(String option, String optionExtraPart) {
403380
trainingInputDir = !"".equals(optionExtraPart) ? optionExtraPart : ".";
404381
} else if ("-html".equals(option)) {
405382
bugReporterType = HTML_REPORTER;
406-
TextUIBugReporter reporter = initializeReporter(optionExtraPart,
407-
() -> new HTMLBugReporter(project, "default.xsl"),
408-
(r, rOpt) -> {
409-
if (!"".equals(rOpt)) {
410-
r.setStylesheet(rOpt);
411-
}
412-
});
413-
if (reporter != null) {
414-
reporters.add(reporter);
383+
HTMLBugReporter htmlBugReporter = new HTMLBugReporter(project, "default.xsl");
384+
optionExtraPart = handleOutputFilePath(htmlBugReporter, optionExtraPart);
385+
if (!"".equals(optionExtraPart)) {
386+
htmlBugReporter.setStylesheet(optionExtraPart);
415387
}
388+
reporters.add(htmlBugReporter);
416389
} else if ("-xdocs".equals(option)) {
417390
bugReporterType = XDOCS_REPORTER;
418-
TextUIBugReporter reporter = initializeReporter(optionExtraPart,
419-
() -> new XDocsBugReporter(project),
420-
(r, rOpts) -> {
421-
});
422-
if (reporter != null) {
423-
reporters.add(reporter);
424-
}
391+
XDocsBugReporter xDocsBugReporter = new XDocsBugReporter(project);
392+
handleOutputFilePath(xDocsBugReporter, optionExtraPart);
393+
reporters.add(xDocsBugReporter);
425394
} else if ("-sarif".equals(option)) {
426395
bugReporterType = SARIF_REPORTER;
427-
TextUIBugReporter reporter = initializeReporter(optionExtraPart,
428-
() -> new SarifBugReporter(project),
429-
(r, rOpts) -> {
430-
});
431-
if (reporter != null) {
432-
reporters.add(reporter);
433-
}
396+
SarifBugReporter sarifBugReporter = new SarifBugReporter(project);
397+
handleOutputFilePath(sarifBugReporter, optionExtraPart);
398+
reporters.add(sarifBugReporter);
434399
} else if ("-applySuppression".equals(option)) {
435400
applySuppression = true;
436401
} else if ("-quiet".equals(option)) {

0 commit comments

Comments
 (0)