Merge pull request #10014 from nextcloud/spotbugs-improvements

Spotbugs improvements

.github/workflows/analysis.yml

@@ -40,6 +40,9 @@ jobs:
                 with:
                     distribution: "temurin"
                     java-version: 11
+            -   name: Install dependencies
+                run: |
+                    python3 -m pip install defusedxml
             -   name: Run analysis wrapper
                 run: |
                     mkdir -p $HOME/.gradle

.gitignore

@@ -50,4 +50,7 @@ fastlane/Fastfile
 **/fastlane/test_output
 /fastlane/vendor/
 /.bundle/
-/fastlane/.bundle/
+/fastlane/.bundle
+
+# python
+**/__pycache__/

app/build.gradle

@@ -430,10 +430,13 @@ tasks.withType(SpotBugsTask){task ->
     excludeFilter = file("${project.rootDir}/spotbugs-filter.xml")
     classes = fileTree("$project.buildDir/intermediates/javac/${variantName}/classes/")
     reports {
-        xml.enabled = false
+        xml {
+            required = true
+        }
         html {
-            enabled = true
-            destination = file("$project.buildDir/reports/spotbugs/spotbugs.html")
+            required = true
+            outputLocation = file("$project.buildDir/reports/spotbugs/spotbugs.html")
+            stylesheet = 'fancy.xsl'
         }
     }
 }

scripts/analysis/analysis-wrapper.sh

@@ -14,8 +14,9 @@ repository="android"
 ruby scripts/analysis/lint-up.rb $1 $2 $3
 lintValue=$?
 
-ruby scripts/analysis/findbugs-up.rb $1 $2 $3
-findbugsValue=$?
+curl 2>/dev/null "https://www.kaminsky.me/nc-dev/$repository-findbugs/$stableBranch.xml" -o "/tmp/$stableBranch.xml"
+ruby scripts/analysis/spotbugs-up.rb "$3"
+spotbugsValue=$?
 
 # exit codes:
 # 0: count was reduced
@@ -25,11 +26,9 @@ findbugsValue=$?
 echo "Branch: $3"
 
 if [ $3 = $stableBranch ]; then
-    echo "New findbugs result for $stableBranch at: https://www.kaminsky.me/nc-dev/$repository-findbugs/$stableBranch.html"
+    echo "New spotbugs result for $stableBranch at: https://www.kaminsky.me/nc-dev/$repository-findbugs/$stableBranch.html"
     curl -u $4:$5 -X PUT https://nextcloud.kaminsky.me/remote.php/webdav/$repository-findbugs/$stableBranch.html --upload-file app/build/reports/spotbugs/spotbugs.html
-
-    summary=$(sed -n "/<h1>Summary<\/h1>/,/<h1>Warnings<\/h1>/p" app/build/reports/spotbugs/spotbugs.html | head -n-1 | sed s'/<\/a>//'g | sed s'/<a.*>//'g | sed s"/Summary/SpotBugs ($stableBranch)/" | tr "\"" "\'" | tr -d "\r\n")
-    curl -u $4:$5 -X PUT -d "$summary" https://nextcloud.kaminsky.me/remote.php/webdav/$repository-findbugs/findbugs-summary-$stableBranch.html
+    curl 2>/dev/null -u "$4:$5" -X PUT "https://nextcloud.kaminsky.me/remote.php/webdav/$repository-findbugs/$stableBranch.xml" --upload-file app/build/reports/spotbugs/gplayDebug.xml
 
     if [ $lintValue -ne 1 ]; then
         echo "New lint result for $stableBranch at: https://www.kaminsky.me/nc-dev/$repository-lint/$stableBranch.html"
@@ -43,7 +42,7 @@ else
     echo "New lint results at https://www.kaminsky.me/nc-dev/$repository-lint/$6.html"
     curl 2>/dev/null -u $4:$5 -X PUT https://nextcloud.kaminsky.me/remote.php/webdav/$repository-lint/$6.html --upload-file app/build/reports/lint/lint.html
 
-    echo "New findbugs results at https://www.kaminsky.me/nc-dev/$repository-findbugs/$6.html"
+    echo "New spotbugs results at https://www.kaminsky.me/nc-dev/$repository-findbugs/$6.html"
     curl 2>/dev/null -u $4:$5 -X PUT https://nextcloud.kaminsky.me/remote.php/webdav/$repository-findbugs/$6.html --upload-file app/build/reports/spotbugs/spotbugs.html
 
     # delete all old comments, starting with Codacy
@@ -65,7 +64,7 @@ else
         checkLibrary=0
     fi
 
-    # lint and findbugs file must exist
+    # lint and spotbugs file must exist
     if [ ! -s app/build/reports/lint/lint.html ] ; then
         echo "lint.html file is missing!"
         exit 1
@@ -108,16 +107,15 @@ else
     fi
 
     lintResult="<h1>Lint</h1><table width='500' cellpadding='5' cellspacing='2'><tr class='tablerow0'><td>Type</td><td><a href='https://www.kaminsky.me/nc-dev/"$repository"-lint/"$stableBranch".html'>$stableBranch</a></td><td><a href='https://www.kaminsky.me/nc-dev/"$repository"-lint/"$6".html'>PR</a></td></tr><tr class='tablerow1'><td>Warnings</td><td>"$lintWarningOld"</td><td>"$lintWarningNew"</td></tr><tr class='tablerow0'><td>Errors</td><td>"$lintErrorOld"</td><td>"$lintErrorNew"</td></tr></table>"
-    findbugsResultNew=$(sed -n "/<h1>Summary<\/h1>/,/<h1>Warnings<\/h1>/p" app/build/reports/spotbugs/spotbugs.html |head -n-1 | sed s'/<\/a>//'g | sed s'/<a.*>//'g | sed s"#Summary#<a href=\"https://www.kaminsky.me/nc-dev/$repository-findbugs/$6.html\">SpotBugs</a> (new)#" | tr "\"" "\'" | tr -d "\n")
-    findbugsResultOld=$(curl 2>/dev/null https://www.kaminsky.me/nc-dev/$repository-findbugs/findbugs-summary-$stableBranch.html | tr "\"" "\'" | tr -d "\r\n" | sed s"#SpotBugs#<a href=\"https://www.kaminsky.me/nc-dev/$repository-findbugs/$stableBranch.html\">SpotBugs</a>#" | tr "\"" "\'" | tr -d "\n")
 
+    spotbugsResult="<h1>SpotBugs</h1>$(scripts/analysis/spotbugsComparison.py "/tmp/$stableBranch.xml" app/build/reports/spotbugs/gplayDebug.xml --link-new "https://www.kaminsky.me/nc-dev/$repository-findbugs/$6.html" --link-base "https://www.kaminsky.me/nc-dev/$repository-findbugs/$stableBranch.html")"
 
     if ( [ $lintValue -eq 1 ] ) ; then
         lintMessage="<h1>Lint increased!</h1>"
     fi
 
-    if ( [ $findbugsValue -eq 1 ] ) ; then
-        findbugsMessage="<h1>SpotBugs increased!</h1>"
+    if ( [ $spotbugsValue -eq 1 ] ) ; then
+        spotbugsMessage="<h1>SpotBugs increased!</h1>"
     fi
 
     # check gplay limitation: all changelog files must only have 500 chars
@@ -132,7 +130,8 @@ else
         notNull="org.jetbrains.annotations.NotNull is used. Please use androidx.annotation.NonNull instead.<br><br>"
     fi
 
-    curl -u $1:$2 -X POST https://api.github.com/repos/nextcloud/android/issues/$7/comments -d "{ \"body\" : \"$codacyResult $lintResult $findbugsResultNew $findbugsResultOld $checkLibraryMessage $lintMessage $findbugsMessage $gplayLimitation $notNull\" }"
+    payload="{ \"body\" : \"$codacyResult $lintResult $spotbugsResult $checkLibraryMessage $lintMessage $spotbugsMessage $gplayLimitation $notNull\" }"
+    curl -u "$1:$2" -X POST "https://api.github.com/repos/nextcloud/android/issues/$7/comments" -d "$payload"
 
     if [ ! -z "$gplayLimitation" ]; then
         exit 1
@@ -150,9 +149,9 @@ else
         exit 1
     fi
 
-    if [ $findbugsValue -eq 2 ]; then
+    if [ $spotbugsValue -eq 2 ]; then
         exit 0
     else
-        exit $findbugsValue
+        exit $spotbugsValue
     fi
 fi

scripts/analysis/findbugs-results.txt

@@ -1 +0,0 @@
-536

scripts/analysis/findbugs-up.rb

@@ -1,128 +0,0 @@
-## Script from https://github.com/tir38/android-lint-entropy-reducer at 07.05.2017
-# adapts to drone, use git username / token as parameter
-
-Encoding.default_external = Encoding::UTF_8
-Encoding.default_internal = Encoding::UTF_8
-
-puts "=================== starting Android FindBugs Entropy Reducer ===================="
-
-# get args
-git_user, git_token, git_branch = ARGV
-
-# ========================  SETUP ============================
-
-# User name for git commits made by this script.
-TRAVIS_GIT_USERNAME = String.new("Drone CI server")
-
-# File name and relative path of generated FindBugs report. Must match build.gradle file:
-#   lintOptions {
-#       htmlOutput file("[FILE_NAME].html")
-#   }
-FINDBUGS_REPORT_FILE = String.new("app/build/reports/spotbugs/spotbugs.html")
-
-# File name and relative path of previous results of this script.
-PREVIOUS_FINDBUGS_RESULTS_FILE=String.new("scripts/analysis/findbugs-results.txt")
-
-# Flag to evaluate warnings. true = check warnings; false = ignore warnings
-CHECK_WARNINGS = true
-
-# File name and relative path to custom FindBugs rules; Can be null or "".
-CUSTOM_FINDBUGS_FILE = String.new("")
-
-# ================ SETUP DONE; DON'T TOUCH ANYTHING BELOW  ================
-
-require 'fileutils'
-require 'pathname'
-require 'open3'
-
-# since we need the xml-simple gem, and we want this script self-contained, let's grab it just when we need it
-begin
-    gem "xml-simple"
-    rescue LoadError
-    system("gem install --user-install xml-simple")
-    Gem.clear_paths
-end
-
-require 'xmlsimple'
-
-# run FindBugs
-puts "running FindBugs..."
-system './gradlew spotbugsGplayDebug 1>/dev/null 2>&1'
-
-# find FindBugs report file
-findbugs_reports = Dir.glob(FINDBUGS_REPORT_FILE)
-if findbugs_reports.length == 0
-    puts "Findbugs HTML report not found."
-    exit 1
-end
-findbugs_report = String.new(findbugs_reports[0])
-
-# find number of warnings
-current_warning_count = `grep -A 3 "<b>Total</b>" app/build/reports/spotbugs/spotbugs.html | tail -n1 | cut -f2 -d">" | cut -f1 -d"<"`.to_i
-puts "found warnings: " + current_warning_count.to_s
-
-# get warning counts from last successful build
-
-previous_results = false
-
-previous_findbugs_reports = Dir.glob(PREVIOUS_FINDBUGS_RESULTS_FILE)
-if previous_findbugs_reports.nil? || previous_findbugs_reports.length == 0
-    previous_findbugs_report = File.new(PREVIOUS_FINDBUGS_RESULTS_FILE, "w") # create for writing to later
-else
-    previous_findbugs_report = String.new(previous_findbugs_reports[0])
-
-    previous_warning_count = File.open(previous_findbugs_report, &:readline).match(/[0-9]*/)[0].to_i
-
-    if previous_warning_count.nil?
-        previous_results = false
-    else
-        previous_results = true
-
-        puts "previous warnings: " + previous_warning_count.to_s
-    end
-end
-
-# compare previous warning count with current warning count
-if previous_results == true && current_warning_count > previous_warning_count
-    puts "FAIL: warning count increased"
-    exit 1
-end
-
-# check if warning and error count stayed the same
-if  previous_results == true && current_warning_count == previous_warning_count
-    puts "SUCCESS: count stayed the same"
-    exit 2
-end
-
-# warning count DECREASED
-puts "SUCCESS: count decreased from " + previous_warning_count.to_s + " to " + current_warning_count.to_s
-
-# write new results to file (will overwrite existing, or create new)
-File.write(previous_findbugs_report, current_warning_count)
-
-# push changes to github (if this script is run locally, we don't want to overwrite git username and email, so save temporarily)
-previous_git_username, _ = Open3.capture2('git config user.name')
-previous_git_username = previous_git_username.strip
-
-previous_git_email, _ = Open3.capture3('git config user.email')
-previous_git_email = previous_git_email.strip
-
-# update git user name and email for this script
-system ("git config --local user.name '"  + git_user + "'")
-system ("git config --local user.email 'android@nextcloud.com'")
-
-# add previous FindBugs result file to git
-system ('git add ' + PREVIOUS_FINDBUGS_RESULTS_FILE)
-
-# commit changes
-system({"GIT_COMMITTER_NAME" => git_user, "GIT_COMMITTER_EMAIL" => "android@nextcloud.com", "GIT_AUTHOR_EMAIL" => "android@nextcloud.com"}, 'git commit -sm "Analysis: update Spotbugs results to reflect reduced error/warning count"')
-
-# push to origin
-system ('git push origin HEAD:' + git_branch)
-
-# restore previous git user name and email
-system("git config --local user.name '#{previous_git_username}'")
-system("git config --local user.email '#{previous_git_email}'")
-
-puts "SUCCESS: count was reduced"
-exit 0 # success

scripts/analysis/spotbugs-up.rb

@@ -0,0 +1,48 @@
+## Script originally from https://github.com/tir38/android-lint-entropy-reducer at 07.05.2017
+# heavily modified since then
+
+Encoding.default_external = Encoding::UTF_8
+Encoding.default_internal = Encoding::UTF_8
+
+puts "=================== starting Android Spotbugs Entropy Reducer ===================="
+
+# get args
+git_branch = ARGV[0]
+
+require 'fileutils'
+require 'pathname'
+require 'open3'
+
+# run Spotbugs
+puts "running Spotbugs..."
+system './gradlew spotbugsGplayDebug 1>/dev/null 2>&1'
+
+# find number of warnings
+current_warning_count = `./scripts/analysis/spotbugsSummary.py --total`.to_i
+puts "found warnings: " + current_warning_count.to_s
+
+# get warning counts from target branch
+previous_xml = "/tmp/#{git_branch}.xml"
+previous_results = File.file?(previous_xml)
+
+if previous_results == true
+    previous_warning_count = `./scripts/analysis/spotbugsSummary.py --total --file #{previous_xml}`.to_i
+    puts "previous warnings: " + previous_warning_count.to_s
+end
+
+# compare previous warning count with current warning count
+if previous_results == true && current_warning_count > previous_warning_count
+    puts "FAIL: warning count increased"
+    exit 1
+end
+
+# check if warning and error count stayed the same
+if  previous_results == true && current_warning_count == previous_warning_count
+    puts "SUCCESS: count stayed the same"
+    exit 0
+end
+
+# warning count DECREASED
+if previous_results == true && current_warning_count < previous_warning_count
+    puts "SUCCESS: count decreased from " + previous_warning_count.to_s + " to " + current_warning_count.to_s
+end

scripts/analysis/spotbugsComparison.py

@@ -0,0 +1,52 @@
+#!/usr/bin/env python3
+import argparse
+import defusedxml.ElementTree as ET
+import spotbugsSummary
+
+
+def print_comparison(old: dict, new: dict, link_base: str, link_new: str):
+    all_keys = sorted(set(list(old.keys()) + list(new.keys())))
+
+    output = "<table><tr><th>Category</th>"
+    old_header = f"<a href='{link_base}'>Base</a>" if link_base is not None else "Base"
+    output += f"<th>{old_header}</th>"
+    new_header = f"<a href='{link_new}'>New</a>" if link_new is not None else "New"
+    output += f"<th>{new_header}</th>"
+    output += "</tr>"
+
+    for category in all_keys:
+        category_count_old = old[category] if category in old else 0
+        category_count_new = new[category] if category in new else 0
+        new_str = f"<b>{category_count_new}</b>" if category_count_new != category_count_old else str(category_count_new)
+        output += "<tr>"
+        output += f"<td>{category}</td>"
+        output += f"<td>{category_count_old}</td>"
+        output += f"<td>{new_str}</td>"
+        output += "</tr>"
+
+    output += "<tr>"
+    output += "<td><b>Total</b></td>"
+    output += f"<td><b>{sum(old.values())}</b></td>"
+    output += f"<td><b>{sum(new.values())}</b></td>"
+    output += "</tr>"
+
+    output += "</table>"
+
+    print(output)
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser()
+    parser.add_argument("base_file", help="base file for comparison")
+    parser.add_argument("new_file", help="new file for comparison")
+    parser.add_argument("--link-base", help="http link to base html report")
+    parser.add_argument("--link-new", help="http link to new html report")
+    args = parser.parse_args()
+
+    base_tree = ET.parse(args.base_file)
+    base_summary = spotbugsSummary.get_counts(base_tree)
+
+    new_tree = ET.parse(args.new_file)
+    new_summary = spotbugsSummary.get_counts(new_tree)
+
+    print_comparison(base_summary, new_summary, args.link_base, args.link_new)

scripts/analysis/spotbugsSummary.py

@@ -0,0 +1,61 @@
+#!/usr/bin/env python3
+import argparse
+import defusedxml.ElementTree as ET
+
+
+def get_counts(tree):
+    category_counts = {}
+    category_names = {}
+    for child in tree.getroot():
+        if child.tag == "BugInstance":
+            category = child.attrib['category']
+            if category in category_counts:
+                category_counts[category] = category_counts[category] + 1
+            else:
+                category_counts[category] = 1
+        elif child.tag == "BugCategory":
+            category = child.attrib['category']
+            category_names[category] = child[0].text
+
+    summary = {}
+    for category in category_counts.keys():
+        summary[category_names[category]] = category_counts[category]
+    return summary
+
+
+def print_html(summary):
+    output = "<table><tr><th>Category</th><th>Count</th></tr>"
+
+    categories = sorted(summary.keys())
+    for category in categories:
+        output += "<tr>"
+        output += f"<td>{category}</td>"
+        output += f"<td>{summary[category]}</td>"
+        output += "</tr>"
+
+    output += "<tr>"
+    output += "<td><b>Total</b></td>"
+    output += f"<td><b>{sum(summary.values())}</b></td>"
+    output += "</tr>"
+
+    output += "</table>"
+
+    print(output)
+
+
+def print_total(summary):
+    print(sum(summary.values()))
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--total", help="print total count instead of summary HTML",
+                        action="store_true")
+    parser.add_argument("--file", help="file to parse", default="app/build/reports/spotbugs/gplayDebug.xml")
+    args = parser.parse_args()
+    tree = ET.parse(args.file)
+    summary = get_counts(tree)
+    if args.total:
+        print_total(summary)
+    else:
+        print_html(summary)