Add ruff as part of PR validation (#72)

Author: debonte

.github/workflows/validation.yml

@@ -30,6 +30,16 @@ jobs:
       - name: Validate pyproject.toml and poetry.lock
         run: poetry check
 
+      - name: Install dependencies
+        run: poetry install
+
+      - name: Validate code formatting
+        run: poetry run ruff format --check
+
+      - name: Validate code style
+        run: poetry run ruff check
+
+
   test:
     if: github.repository == 'microsoft/sarif-tools'
     runs-on: ubuntu-latest

.vscode/extensions.json

@@ -0,0 +1,9 @@
+{
+    // See https://go.microsoft.com/fwlink/?LinkId=827846
+    // for the documentation about the extensions.json format
+    "recommendations": [
+        "charliermarsh.ruff",
+        "ms-python.python",
+        "ms-python.vscode-pylance"
+    ]
+}

poetry.lock

@@ -26,11 +26,11 @@ python-docx = "^1.1.2"
 pyyaml = "^6.0.1"
 
 [tool.poetry.dev-dependencies]
-black = "^24.3.0"
 jsonschema = "^4.23.0"
 pylint = "^3.2"
 pytest = "^8.3"
 pytest-cov = "^5.0"
+ruff = "^0.6.8"
 
 [tool.poetry.scripts]
 sarif = "sarif.cmdline.main:main"

pyproject.toml

@@ -113,14 +113,14 @@ def _run_git_blame_on_files(files_to_blame, repo_path):
         with subprocess.Popen(cmd, stdout=subprocess.PIPE, cwd=repo_path) as proc:
             blame_info = {"commits": {}, "line_to_commit": {}}
             file_blame_info[file_path] = blame_info
-            reading_commit_headers = False
+            commit_hash: str | None = None
             for line_bytes in proc.stdout.readlines():
                 # Convert byte sequence to string and remove trailing LF
                 line_string = line_bytes.decode("utf-8")[:-1]
                 # Now parse output from git blame --porcelain
-                if reading_commit_headers:
+                if commit_hash:
                     if line_string.startswith("\t"):
-                        reading_commit_headers = False
+                        commit_hash = None
                         # Ignore line contents = source code
                     elif " " in line_string:
                         space_pos = line_string.index(" ")
@@ -137,7 +137,7 @@ def _run_git_blame_on_files(files_to_blame, repo_path):
                     commit_line = commit_line_info[2]
                     blame_info["commits"].setdefault(commit_hash, {})
                     blame_info["line_to_commit"][commit_line] = commit_hash
-                    reading_commit_headers = True
+
             # Ensure process terminates
             proc.communicate()
             if proc.returncode:

sarif/operations/blame_op.py

@@ -32,7 +32,7 @@ def generate_summary(
                 output_file_name,
             )
             with open(output_file, "w", encoding="utf-8") as file_out:
-                file_out.writelines(l + "\n" for l in summary_lines)
+                file_out.writelines(line + "\n" for line in summary_lines)
         output_file_name = "static_analysis_summary.txt"
         output_file = os.path.join(output, output_file_name)
 
@@ -45,7 +45,7 @@ def generate_summary(
             output_file,
         )
         with open(output_file, "w", encoding="utf-8") as file_out:
-            file_out.writelines(l + "\n" for l in summary_lines)
+            file_out.writelines(line + "\n" for line in summary_lines)
     else:
         for lstr in summary_lines:
             print(lstr)

sarif/operations/summary_op.py

@@ -168,13 +168,15 @@ def read_result_severity(result, run) -> Literal["none", "note", "warning", "err
         # Honor the invocation's configuration override if present...
         invocation = read_result_invocation(result, run)
         if invocation:
-            ruleConfigurationOverrides = invocation.get("ruleConfigurationOverrides", [])
+            ruleConfigurationOverrides = invocation.get(
+                "ruleConfigurationOverrides", []
+            )
             override = next(
                 (
                     override
                     for override in ruleConfigurationOverrides
-                    if override.get("descriptor", {}).get("id") == rule.get("id") or
-                       override.get("descriptor", {}).get("index") == ruleIndex
+                    if override.get("descriptor", {}).get("id") == rule.get("id")
+                    or override.get("descriptor", {}).get("index") == ruleIndex
                 ),
                 None,
             )

sarif/sarif_file_utils.py

@@ -141,9 +141,7 @@
 
 def test_diff_issues_reordered():
     mtime = datetime.datetime.now()
-    sarif = sarif_file.SarifFile(
-        "SARIF", SARIF, mtime=mtime
-    )
+    sarif = sarif_file.SarifFile("SARIF", SARIF, mtime=mtime)
     sarif_reordered = sarif_file.SarifFile(
         "SARIF_WITH_ISSUES_REORDERED", SARIF_WITH_ISSUES_REORDERED, mtime=mtime
     )

tests/diff/test_diff_issues_reordered.py

@@ -1,5 +1,5 @@
 import pytest
-from sarif.filter.general_filter import GeneralFilter, PropertyFilter, load_filter_file
+from sarif.filter.general_filter import GeneralFilter, load_filter_file
 from sarif.filter.filter_stats import load_filter_stats_from_json
 
 

tests/test_general_filter.py

@@ -38,12 +38,16 @@ def test_combine_code_and_description_long_code():
 
 
 def test_read_result_rule():
-    run = {"tool":
-           {"driver":
-            {"rules": [
-                {"id": "id0", "defaultConfiguration": {"level": "none"}},
-                {"id": "id1", "defaultConfiguration": {"level": "error"}}
-             ]}}}
+    run = {
+        "tool": {
+            "driver": {
+                "rules": [
+                    {"id": "id0", "defaultConfiguration": {"level": "none"}},
+                    {"id": "id1", "defaultConfiguration": {"level": "error"}},
+                ]
+            }
+        }
+    }
     rule_id0 = run["tool"]["driver"]["rules"][0]
     rule_id1 = run["tool"]["driver"]["rules"][1]
 
@@ -57,7 +61,7 @@ def test_read_result_rule():
     assert rule == rule_id1
     assert ruleIndex == 1
 
-    result = {"rule": { "index": 1}}
+    result = {"rule": {"index": 1}}
     (rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
     assert rule == rule_id1
     assert ruleIndex == 1
@@ -67,7 +71,7 @@ def test_read_result_rule():
     assert rule == rule_id1
     assert ruleIndex == 1
 
-    result = {"rule": { "id": "id1"}}
+    result = {"rule": {"id": "id1"}}
     (rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
     assert rule == rule_id1
     assert ruleIndex == 1
@@ -84,10 +88,7 @@ def test_read_result_rule():
 
 
 def test_read_result_invocation():
-    run = {"invocations": [
-             {"foo": 1},
-             {"bar": "baz"}
-           ]}
+    run = {"invocations": [{"foo": 1}, {"bar": "baz"}]}
 
     result = {}
     invocation = sarif_file_utils.read_result_invocation(result, run)
@@ -124,19 +125,29 @@ def test_read_result_severity():
     severity = sarif_file_utils.read_result_severity(result, {})
     assert severity == "none"
 
-    run = {"invocations": [
-             {"ruleConfigurationOverrides": [ {"descriptor": {"id": "id1"}, "configuration": {"level": "note"}} ]},
-             {"ruleConfigurationOverrides": [ {"descriptor": {"index": 1}, "configuration": {"level": "note"}} ]},
-             { }
-           ],
-           "tool":
-             {"driver":
-               {"rules": [
-                 {"id": "id0", "defaultConfiguration": {"level": "none"}},
-                 {"id": "id1", "defaultConfiguration": {"level": "error"}}
-               ]}
-             }
-           }
+    run = {
+        "invocations": [
+            {
+                "ruleConfigurationOverrides": [
+                    {"descriptor": {"id": "id1"}, "configuration": {"level": "note"}}
+                ]
+            },
+            {
+                "ruleConfigurationOverrides": [
+                    {"descriptor": {"index": 1}, "configuration": {"level": "note"}}
+                ]
+            },
+            {},
+        ],
+        "tool": {
+            "driver": {
+                "rules": [
+                    {"id": "id0", "defaultConfiguration": {"level": "none"}},
+                    {"id": "id1", "defaultConfiguration": {"level": "error"}},
+                ]
+            }
+        },
+    }
 
     # If kind has the value "fail" and level is absent, then level SHALL be determined by the following procedure:
     # IF rule is present THEN
@@ -167,15 +178,15 @@ def test_read_result_severity():
     severity = sarif_file_utils.read_result_severity(result, run)
     assert severity == "error"
 
-    result = {"rule": { "index": 1}}
+    result = {"rule": {"index": 1}}
     severity = sarif_file_utils.read_result_severity(result, run)
     assert severity == "error"
 
     result = {"ruleId": "id1"}
     severity = sarif_file_utils.read_result_severity(result, run)
     assert severity == "error"
 
-    result = {"rule": { "id": "id1"}}
+    result = {"rule": {"id": "id1"}}
     severity = sarif_file_utils.read_result_severity(result, run)
     assert severity == "error"