diff --git a/CHANGELOG.md b/CHANGELOG.md index 89e573f5924..1d5cab2e1fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. - Updated documentation by adding parenthesis `()` to the negative odd check message ([#1995](https://github.com/spotbugs/spotbugs/issues/1995)) ### Fixed +- Fixed reports to truncate existing files before writing new content ([#1950](https://github.com/spotbugs/spotbugs/issues/1950)) - Bumped Saxon-HE from 10.6 to 11.3 ([#1955](https://github.com/spotbugs/spotbugs/pull/1955), [#1999](https://github.com/spotbugs/spotbugs/pull/1999)) - Fixed traversal of nested archives governed by `-nested:true` ([#1930](https://github.com/spotbugs/spotbugs/pull/1930)) - Warnings of deprecated System::setSecurityManager calls on Java 17 ([#1983](https://github.com/spotbugs/spotbugs/pull/1983)) @@ -23,6 +24,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. * `THROWS_METHOD_THROWS_CLAUSE_THROWABLE` is reported when a method has Throwable in its throws clause (See [SEI CERT ERR07-J](https://wiki.sei.cmu.edu/confluence/display/java/ERR07-J.+Do+not+throw+RuntimeException%2C+Exception%2C+or+Throwable)) * New rule `PERM_SUPER_NOT_CALLED_IN_GETPERMISSIONS` to warn for custom class loaders who do not call their superclasses' `getPermissions()` in their `getPermissions()` method. This rule based on the SEI CERT rule *SEC07-J Call the superclass's getPermissions() method when writing a custom class loader*. ([#SEC07-J](https://wiki.sei.cmu.edu/confluence/display/java/SEC07-J.+Call+the+superclass%27s+getPermissions%28%29+method+when+writing+a+custom+class+loader)) * New rule `USC_POTENTIAL_SECURITY_CHECK_BASED_ON_UNTRUSTED_SOURCE` to detect cases where a non-final method of a non-final class is called from public methods of public classes and then the same method is called on the same object inside a doPrivileged block. Since the called method may have been overridden to behave differently on the first and second invocations this is a possible security check based on an unreliable source. This rule is based on *SEC02-J. Do not base security checks on untrusted sources*. ([#SEC02-J](https://wiki.sei.cmu.edu/confluence/display/java/SEC02-J.+Do+not+base+security+checks+on+untrusted+sources)) +* New detector `DontUseFloatsAsLoopCounters` to detect usage of floating-point variables as loop counters (`FL_FLOATS_AS_LOOP_COUNTERS`), according to SEI CERT rules [NUM09-J. Do not use floating-point variables as loop counters](https://wiki.sei.cmu.edu/confluence/display/java/NUM09-J.+Do+not+use+floating-point+variables+as+loop+counters) * New test detector `ViewCFG` to visualize the control-flow graph for `SpotBugs` developers ## 4.6.0 - 2022-03-08 diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts index d6bf36812d7..cd91adf4480 100644 --- a/buildSrc/build.gradle.kts +++ b/buildSrc/build.gradle.kts @@ -6,5 +6,5 @@ repositories { gradlePluginPortal() } dependencies { - implementation("com.diffplug.gradle:goomph:3.36.0") + implementation("com.diffplug.gradle:goomph:3.36.2") } diff --git a/eclipsePlugin-junit/build.gradle b/eclipsePlugin-junit/build.gradle index 18857ef56a8..b15418cb6cb 100644 --- a/eclipsePlugin-junit/build.gradle +++ b/eclipsePlugin-junit/build.gradle @@ -12,7 +12,7 @@ tasks.named('compileJava', JavaCompile).configure { dependencies { implementation project(':eclipsePlugin') testImplementation 'junit:junit:4.13.2' - testImplementation 'org.mockito:mockito-core:4.5.0' + testImplementation 'org.mockito:mockito-core:4.5.1' } tasks.named('jacocoTestReport', JacocoReport).configure { diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/TextUICommandLineTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/TextUICommandLineTest.java index dccd8d277c4..aef57c5192e 100644 --- a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/TextUICommandLineTest.java +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/TextUICommandLineTest.java @@ -36,6 +36,19 @@ public void handleOutputFilePathUsesGzip() throws IOException { assertThat("GZip file should have -117 as its header", written[1], is((byte) -117)); } + @Test + public void handleOutputFileTruncatesExisting() throws IOException { + Path file = Files.createTempFile("spotbugs", ".html"); + Files.writeString(file, "content"); + TextUICommandLine commandLine = new TextUICommandLine(); + SortingBugReporter reporter = new SortingBugReporter(); + commandLine.handleOutputFilePath(reporter, "withMessages=" + file.toFile().getAbsolutePath()); + + reporter.finish(); + byte[] written = Files.readAllBytes(file); + assertThat("Output file should be truncated to 0 bytes", written.length, is(0)); + } + @Test public void htmlReportWithOption() throws IOException { Path xmlFile = Files.createTempFile("spotbugs", ".xml"); diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/DontUseFloatsAsLoopCountersTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/DontUseFloatsAsLoopCountersTest.java new file mode 100644 index 00000000000..73df822f555 --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/DontUseFloatsAsLoopCountersTest.java @@ -0,0 +1,38 @@ +package edu.umd.cs.findbugs.detect; + +import org.junit.Test; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.hamcrest.Matchers.hasItem; +import static org.junit.Assert.assertThat; + +public class DontUseFloatsAsLoopCountersTest extends AbstractIntegrationTest { + @Test + public void testChecks() { + performAnalysis("DontUseFloatsAsLoopCounters.class"); + assertNumOfEOSBugs(3); + assertBug("main", 5); + assertBug("main", 9); + assertBug("main", 12); + } + + private void assertBug(String method, int line) { + final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder() + .bugType("FL_FLOATS_AS_LOOP_COUNTERS") + .inClass("DontUseFloatsAsLoopCounters") + .inMethod(method) + .atLine(line) + .build(); + assertThat(getBugCollection(), hasItem(bugInstanceMatcher)); + } + + private void assertNumOfEOSBugs(int num) { + final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder() + .bugType("FL_FLOATS_AS_LOOP_COUNTERS").build(); + assertThat(getBugCollection(), containsExactly(num, bugTypeMatcher)); + } +} diff --git a/spotbugs/etc/findbugs.xml b/spotbugs/etc/findbugs.xml index 1007dbd6e50..8d60497ae13 100644 --- a/spotbugs/etc/findbugs.xml +++ b/spotbugs/etc/findbugs.xml @@ -661,6 +661,8 @@ reports="MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR,MC_OVERRIDABLE_METHOD_CALL_IN_CLONE"/> + + diff --git a/spotbugs/etc/messages.xml b/spotbugs/etc/messages.xml index 48fe3ead07e..249492647ba 100644 --- a/spotbugs/etc/messages.xml +++ b/spotbugs/etc/messages.xml @@ -1746,6 +1746,13 @@ factory pattern to create these objects.

]]> + +
+ Checks for floats in loop counters.

+ ]]> +
+

]]>
+ + Do not use floating-point variables as loop counters + Using floating-point loop counters can lead to unexpected behavior. +
+ +Using floating-point variables should not be used as loop counters, as they are not precise, which may result in incorrect loops. A loop counter is a variable that is changed with each iteration and controls when the loop should terminate. It is decreased or increased by a fixed amount each iteration.

+

See rule NUM09-J.

+]]> +
+ Method intentionally throws RuntimeException. Method intentionally throws RuntimeException. @@ -8707,6 +8724,7 @@ object explicitly.

Method lists Exception in its throws clause.
+ When declaring a method, the types of exceptions in the throws clause should be the most specific. Therefore, using Exception in the throws clause would force the caller to either use it in its own throws clause, or use it in a try-catch block (when it does not necessarily contain any meaningful information about the thrown exception).

diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/TextUICommandLine.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/TextUICommandLine.java index 88a9c509cda..a834bb0004e 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/TextUICommandLine.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/TextUICommandLine.java @@ -278,7 +278,8 @@ public boolean justPrintVersion() { if (index >= 0) { Path path = Paths.get(optionExtraPart.substring(index + 1)); try { - OutputStream oStream = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + OutputStream oStream = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING); if ("gz".equals(Util.getFileExtension(path.toFile()))) { oStream = new GZIPOutputStream(oStream); } diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DontUseFloatsAsLoopCounters.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DontUseFloatsAsLoopCounters.java new file mode 100644 index 00000000000..75f17ab4973 --- /dev/null +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/DontUseFloatsAsLoopCounters.java @@ -0,0 +1,28 @@ +package edu.umd.cs.findbugs.detect; + +import org.apache.bcel.Const; + +import edu.umd.cs.findbugs.BugInstance; +import edu.umd.cs.findbugs.BugReporter; +import edu.umd.cs.findbugs.StatelessDetector; +import edu.umd.cs.findbugs.bcel.OpcodeStackDetector; +import java.util.stream.Stream; + +public class DontUseFloatsAsLoopCounters extends OpcodeStackDetector implements StatelessDetector { + + private final BugReporter bugReporter; + + public DontUseFloatsAsLoopCounters(BugReporter bugReporter) { + this.bugReporter = bugReporter; + } + + @Override + public void sawOpcode(int seen) { + if ((seen == Const.GOTO || seen == Const.GOTO_W) && + Stream.of(Const.FLOAD, Const.FLOAD_0, Const.FLOAD_1, Const.FLOAD_2, Const.FLOAD_3, Const.DLOAD, Const.DLOAD_0, Const.DLOAD_1, + Const.DLOAD_2, Const.DLOAD_3).anyMatch(x -> x == getCodeByte(getBranchTarget())) && (getBranchTarget() < getPC())) { + bugReporter.reportBug(new BugInstance(this, "FL_FLOATS_AS_LOOP_COUNTERS", NORMAL_PRIORITY) + .addClassAndMethod(this).addSourceLine(this, getBranchTarget())); + } + } +} diff --git a/spotbugsTestCases/build.gradle b/spotbugsTestCases/build.gradle index aa8b4c1a34a..ce9177a7d6d 100644 --- a/spotbugsTestCases/build.gradle +++ b/spotbugsTestCases/build.gradle @@ -24,7 +24,7 @@ dependencies { api 'net.jcip:jcip-annotations:1.0' implementation 'org.springframework:spring-core:5.3.19' compileOnly 'javax.annotation:javax.annotation-api:1.3.2' - implementation 'org.checkerframework:checker-qual:3.21.4' + implementation 'org.checkerframework:checker-qual:3.22.0' implementation 'junit:junit:4.13.2' implementation 'org.testng:testng:7.5' diff --git a/spotbugsTestCases/src/java/DontUseFloatsAsLoopCounters.java b/spotbugsTestCases/src/java/DontUseFloatsAsLoopCounters.java new file mode 100644 index 00000000000..6122669e199 --- /dev/null +++ b/spotbugsTestCases/src/java/DontUseFloatsAsLoopCounters.java @@ -0,0 +1,34 @@ +class DontUseFloatsAsLoopCounters { + public static void main(String[] args) { + //noncompliant + float x = 0.1f; + while (x<10){ + System.out.println(x); + x++; + } + for (float y = 0.2f; y <= 1.0f; y += 0.1f) { + System.out.println(y); + } + for (double d = 0.2d; d <= 1.0d; d += 0.1d) { + System.out.println(d); + } + //compliant + for (int count = 1; count <= 10; count += 1) { + float q = count/10.0f; + System.out.println(q); + System.out.println(count); + } + int c = 0; + while (c<5){ + c++; + } + boolean b = true; + while (b){ + b = false; + } + int p = 1; + while (p<9){ + p*=2; + } +} +} \ No newline at end of file