Implement spotbugs linter aspect (#442)

* Add Spotbugs Co-authored-by: Farid Zakaria <[email protected]> Co-authored-by: Vince Rose<[email protected]> Co-authored-by: Na Lou <[email protected]> * use remotejdk_11 * remove unused deps * move spotbugs java_binary to userspace * fix style issue
aspect-build · Dec 17, 2024 · b4c90f0 · b4c90f0
1 parent 76a8af0
commit b4c90f0
Show file tree

Hide file tree

Showing 16 changed files with 376 additions and 33 deletions.
diff --git a/README.md b/README.md
@@ -21,38 +21,39 @@ Features:
 
 New tools are being added frequently, so check this page again!
 
-| Language               | Formatter             | Linter(s)            |
-| ---------------------- | --------------------- | -------------------- |
-| C / C++                | [clang-format]        | [clang-tidy]         |
-| Cuda                   | [clang-format]        |                      |
-| CSS, Less, Sass        | [Prettier]            | [Stylelint]          |
-| Go                     | [gofmt] or [gofumpt]  |                      |
-| GraphQL                | [Prettier]            |                      |
-| HCL (Hashicorp Config) | [terraform] fmt       |                      |
-| HTML                   | [Prettier]            |                      |
-| JSON                   | [Prettier]            |                      |
-| Java                   | [google-java-format]  | [pmd] , [Checkstyle] |
-| JavaScript             | [Prettier]            | [ESLint]             |
-| Jsonnet                | [jsonnetfmt]          |                      |
-| Kotlin                 | [ktfmt]               | [ktlint]             |
-| Markdown               | [Prettier]            | [Vale]               |
-| Protocol Buffer        | [buf]                 | [buf lint]           |
-| Python                 | [ruff]                | [flake8], [ruff]     |
-| Rust                   | [rustfmt]             |                      |
-| SQL                    | [prettier-plugin-sql] |                      |
-| Scala                  | [scalafmt]            |                      |
-| Shell                  | [shfmt]               | [shellcheck]         |
-| Starlark               | [Buildifier]          |                      |
-| Swift                  | [SwiftFormat] (1)     |                      |
-| TSX                    | [Prettier]            | [ESLint]             |
-| TypeScript             | [Prettier]            | [ESLint]             |
-| YAML                   | [yamlfmt]             |                      |
+| Language               | Formatter             | Linter(s)                        |
+| ---------------------- | --------------------- |----------------------------------|
+| C / C++                | [clang-format]        | [clang-tidy]                     |
+| Cuda                   | [clang-format]        |                                  |
+| CSS, Less, Sass        | [Prettier]            | [Stylelint]                      |
+| Go                     | [gofmt] or [gofumpt]  |                                  |
+| GraphQL                | [Prettier]            |                                  |
+| HCL (Hashicorp Config) | [terraform] fmt       |                                  |
+| HTML                   | [Prettier]            |                                  |
+| JSON                   | [Prettier]            |                                  |
+| Java                   | [google-java-format]  | [pmd] , [Checkstyle], [Spotbugs] |
+| JavaScript             | [Prettier]            | [ESLint]                         |
+| Jsonnet                | [jsonnetfmt]          |                                  |
+| Kotlin                 | [ktfmt]               | [ktlint]                         |
+| Markdown               | [Prettier]            | [Vale]                           |
+| Protocol Buffer        | [buf]                 | [buf lint]                       |
+| Python                 | [ruff]                | [flake8], [ruff]                 |
+| Rust                   | [rustfmt]             |                                  |
+| SQL                    | [prettier-plugin-sql] |                                  |
+| Scala                  | [scalafmt]            |                                  |
+| Shell                  | [shfmt]               | [shellcheck]                     |
+| Starlark               | [Buildifier]          |                                  |
+| Swift                  | [SwiftFormat] (1)     |                                  |
+| TSX                    | [Prettier]            | [ESLint]                         |
+| TypeScript             | [Prettier]            | [ESLint]                         |
+| YAML                   | [yamlfmt]             |                                  |
 
 [prettier]: https://prettier.io
 [google-java-format]: https://github.com/google/google-java-format
 [flake8]: https://flake8.pycqa.org/en/latest/index.html
 [pmd]: https://docs.pmd-code.org/latest/index.html
 [checkstyle]: https://checkstyle.sourceforge.io/cmdline.html
+[spotbugs]: https://spotbugs.github.io/
 [buf lint]: https://buf.build/docs/lint/overview
 [eslint]: https://eslint.org/
 [swiftformat]: https://github.com/nicklockwood/SwiftFormat

diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel
@@ -32,6 +32,11 @@ stardoc_with_diff_test(
     bzl_library_target = "//lint:checkstyle",
 )
 
+stardoc_with_diff_test(
+    name = "spotbugs",
+    bzl_library_target = "//lint:spotbugs",
+)
+
 stardoc_with_diff_test(
     name = "format",
     bzl_library_target = "//format:defs",

diff --git a/docs/spotbugs.md b/docs/spotbugs.md
diff --git a/example/.aspect/cli/config.yaml b/example/.aspect/cli/config.yaml
@@ -10,3 +10,4 @@ lint:
     - //tools/lint:linters.bzl%vale
     - //tools/lint:linters.bzl%checkstyle
     - //tools/lint:linters.bzl%clang_tidy
+    - //tools/lint:linters.bzl%spotbugs
diff --git a/example/.bazelrc b/example/.bazelrc
@@ -1,9 +1,33 @@
 # Automatically apply --config=linux, --config=windows etc
 common --enable_platform_specific_config
 
-# Don't depend on a JAVA_HOME pointing at a system JDK
+# Aspect recommended Bazel flags when using rules_java and rules_jvm_external
+
+# Pin java versions to desired language level
+# See https://bazel.build/docs/bazel-and-java#java-versions
+# and https://en.wikipedia.org/wiki/Java_version_history
+
+# What version of Java are the source files in this repo?
+# See https://bazel.build/docs/user-manual#java-language-version
+common --java_language_version=17
+
+# The Java language version used to build tools that are executed during a build
+# See https://bazel.build/docs/user-manual#tool-java-language-version
+common --tool_java_language_version=17
+
+# The version of JVM to use to execute the code and run the tests.
+# NB: The default value is local_jdk which is non-hermetic.
+# See https://bazel.build/docs/user-manual#java-runtime-version
+common --java_runtime_version=remotejdk_17
+
+# The version of JVM used to execute tools that are needed during a build.
+# See https://bazel.build/docs/user-manual#tool-java-runtime-version
+common --tool_java_runtime_version=remotejdk_17
+
+# Repository rules, such as rules_jvm_external: put Bazel's JDK on the path.
+# Avoids non-hermeticity from dependency on a JAVA_HOME pointing at a system JDK
 # see https://github.com/bazelbuild/rules_jvm_external/issues/445
-build --repo_env=JAVA_HOME=../bazel_tools/jdk
+common --repo_env=JAVA_HOME=../bazel_tools/jdk
 
 common --incompatible_enable_proto_toolchain_resolution
 common --@aspect_rules_ts//ts:skipLibCheck=always

diff --git a/example/BUILD.bazel b/example/BUILD.bazel
@@ -25,6 +25,7 @@ exports_files(
         ".editorconfig",
         "ktlint-baseline.xml",
         ".clang-tidy",
+        "spotbugs-exclude.xml",
     ],
     visibility = ["//visibility:public"],
 )

diff --git a/example/MODULE.bazel b/example/MODULE.bazel
@@ -9,8 +9,8 @@ bazel_dep(name = "rules_buf", version = "0.3.0")
 bazel_dep(name = "bazel_skylib", version = "1.4.2")
 bazel_dep(name = "toolchains_llvm", version = "0.10.3")
 bazel_dep(name = "toolchains_protoc", version = "0.3.0")
-bazel_dep(name = "rules_java", version = "5.5.0")
-bazel_dep(name = "rules_jvm_external", version = "4.5")
+bazel_dep(name = "rules_java", version = "8.5.0")
+bazel_dep(name = "rules_jvm_external", version = "6.5")
 bazel_dep(name = "rules_go", version = "0.42.0", repo_name = "io_bazel_rules_go")
 bazel_dep(name = "rules_proto", version = "6.0.0")
 bazel_dep(name = "rules_python", version = "0.26.0")

diff --git a/example/WORKSPACE.bazel b/example/WORKSPACE.bazel
@@ -318,6 +318,10 @@ load("@aspect_rules_lint//lint:ktlint.bzl", "fetch_ktlint")
 
 fetch_ktlint()
 
+load("@aspect_rules_lint//lint:spotbugs.bzl", "fetch_spotbugs")
+
+fetch_spotbugs()
+
 ########################
 # Optional: multitool provides defaults for some tools such as yamlfmt
 # If you do not set up multitool, you must provide these tools yourself

diff --git a/example/WORKSPACE.bzlmod b/example/WORKSPACE.bzlmod
@@ -31,3 +31,7 @@ fetch_vale()
 load("@aspect_rules_lint//lint:ktlint.bzl", "fetch_ktlint")
 
 fetch_ktlint()
+
+load("@aspect_rules_lint//lint:spotbugs.bzl", "fetch_spotbugs")
+
+fetch_spotbugs()
diff --git a/example/lint.sh b/example/lint.sh
@@ -37,7 +37,7 @@ if [ $machine == "Windows" ]; then
 	# avoid missing linters on windows platform
 	args=("--aspects=$(echo //tools/lint:linters.bzl%{flake8,pmd,ruff,vale,clang_tidy} | tr ' ' ',')")
 else
-	args=("--aspects=$(echo //tools/lint:linters.bzl%{buf,eslint,flake8,ktlint,pmd,ruff,shellcheck,stylelint,vale,clang_tidy} | tr ' ' ',')")
+	args=("--aspects=$(echo //tools/lint:linters.bzl%{buf,eslint,flake8,ktlint,pmd,ruff,shellcheck,stylelint,vale,clang_tidy,spotbugs} | tr ' ' ',')")
 fi
 
 # NB: perhaps --remote_download_toplevel is needed as well with remote execution?

diff --git a/example/spotbugs-exclude.xml b/example/spotbugs-exclude.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<FindBugsFilter>
+    <!-- Exclude all warnings for the Foo.readFile() method -->
+    <Match>
+        <Bug code="OS" />
+    </Match>
+</FindBugsFilter>
diff --git a/example/src/Foo.java b/example/src/Foo.java
@@ -4,7 +4,31 @@
 // "category/java/errorprone.xml/FinalizeOverloaded"
 // src/equals_null.java:6: FinalizeOverloaded:     Finalize methods should not be overloaded
 // Error: bazel exited with exit code: 4
+
+//spotbugs result
+//M B OS: Foo.readFile() may fail to close stream  At Foo.java:[line 18]
+//H D DLS: Dead store to $L1 in Foo.readFile()  At Foo.java:[line 18]
+//M X OBL: Foo.readFile() may fail to clean up java.io.InputStream  Obligation to clean up resource created at Foo.java:[line 18] is not discharged
 public class Foo {
-  // this is confusing and probably a bug
-  protected void finalize(int a) {}
+
+  // SpotBugs violation: Logical errors (NP_NULL_ON_SOME_PATH + DLS_DEAD_STORE)
+  public void someMethod(String str) {
+    if (str.equals("test")) { // Possible NPE if str is null
+      System.out.println("Valid string");
+    }
+
+    int a = 5;
+    a = 10; // The assignment to 5 is dead (overwritten with 10)
+    System.out.println(a);
+  }
+
+  // SpotBugs violation: Resource leak (RCN_RESOURCE_LEAK)
+  public void readFile() {
+    try {
+      java.io.FileInputStream fis = new java.io.FileInputStream("somefile.txt");
+      // FileInputStream not closed, causing resource leak
+    } catch (java.io.IOException e) {
+      e.printStackTrace();
+    }
+  }
 }
diff --git a/example/tools/lint/BUILD.bazel b/example/tools/lint/BUILD.bazel
@@ -46,6 +46,21 @@ java_binary(
     runtime_deps = ["@com_puppycrawl_tools_checkstyle//jar"],
 )
 
+java_binary(
+    name = "spotbugs",
+    jvm_flags = [
+        "--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+        "--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+        "--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+        "--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+        "--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+    ],
+    main_class = "edu.umd.cs.findbugs.LaunchAppropriateUI",
+    runtime_deps = [
+        "@spotbugs//:jar",
+    ],
+)
+
 native_binary(
     name = "vale",
     src = select(

diff --git a/example/tools/lint/linters.bzl b/example/tools/lint/linters.bzl
@@ -10,6 +10,7 @@ load("@aspect_rules_lint//lint:lint_test.bzl", "lint_test")
 load("@aspect_rules_lint//lint:pmd.bzl", "lint_pmd_aspect")
 load("@aspect_rules_lint//lint:ruff.bzl", "lint_ruff_aspect")
 load("@aspect_rules_lint//lint:shellcheck.bzl", "lint_shellcheck_aspect")
+load("@aspect_rules_lint//lint:spotbugs.bzl", "lint_spotbugs_aspect")
 load("@aspect_rules_lint//lint:stylelint.bzl", "lint_stylelint_aspect")
 load("@aspect_rules_lint//lint:vale.bzl", "lint_vale_aspect")
 
@@ -110,3 +111,10 @@ clang_tidy_global_config = lint_clang_tidy_aspect(
     angle_includes_are_system = False,
     verbose = False,
 )
+
+spotbugs = lint_spotbugs_aspect(
+    binary = "@@//tools/lint:spotbugs",
+    exclude_filter = "@@//:spotbugs-exclude.xml",
+)
+
+spotbugs_test = lint_test(aspect = spotbugs)
diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel
@@ -179,6 +179,15 @@ bzl_library(
     ],
 )
 
+bzl_library(
+    name = "spotbugs",
+    srcs = ["spotbugs.bzl"],
+    visibility = ["//visibility:public"],
+    deps = _BAZEL_TOOLS + [
+        "//lint/private:lint_aspect",
+    ],
+)
+
 bzl_library(
     name = "ruff",
     srcs = ["ruff.bzl"],