From 9bd9981e709fddfd8f2f7d1d5d7c7a7ed1727136 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 13 Sep 2024 20:05:45 +0530 Subject: [PATCH] Create insta snapshot for SARIF output (#13345) ## Summary Follow-up from #13268, this PR updates the test case to use `assert_snapshot` now that the output is limited to only include the rules with diagnostics. ## Test Plan `cargo insta test` --- Cargo.lock | 118 ++++++++++++++ crates/ruff_linter/Cargo.toml | 2 +- crates/ruff_linter/src/message/sarif.rs | 32 +--- ...inter__message__sarif__tests__results.snap | 146 ++++++++++++++++++ 4 files changed, 271 insertions(+), 27 deletions(-) create mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap diff --git a/Cargo.lock b/Cargo.lock index c125563ae52fe..491d2094eb1d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -194,6 +194,15 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" +[[package]] +name = "block-buffer" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +dependencies = [ + "generic-array", +] + [[package]] name = "bstr" version = "1.10.0" @@ -511,6 +520,15 @@ dependencies = [ "rustc-hash 1.1.0", ] +[[package]] +name = "cpufeatures" +version = "0.2.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51e852e6dc9a5bed1fae92dd2375037bf2b768725bf3be87811edee3249d09ad" +dependencies = [ + "libc", +] + [[package]] name = "crc32fast" version = "1.4.0" @@ -616,6 +634,16 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" +[[package]] +name = "crypto-common" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" +dependencies = [ + "generic-array", + "typenum", +] + [[package]] name = "ctrlc" version = "3.4.5" @@ -694,6 +722,16 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "digest" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "block-buffer", + "crypto-common", +] + [[package]] name = "dirs" version = "4.0.0" @@ -879,6 +917,16 @@ dependencies = [ "libc", ] +[[package]] +name = "generic-array" +version = "0.14.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +dependencies = [ + "typenum", + "version_check", +] + [[package]] name = "getopts" version = "0.2.21" @@ -1112,6 +1160,8 @@ dependencies = [ "globset", "lazy_static", "linked-hash-map", + "pest", + "pest_derive", "regex", "serde", "similar", @@ -1707,6 +1757,51 @@ version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e" +[[package]] +name = "pest" +version = "2.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd53dff83f26735fdc1ca837098ccf133605d794cdae66acfc2bfac3ec809d95" +dependencies = [ + "memchr", + "thiserror", + "ucd-trie", +] + +[[package]] +name = "pest_derive" +version = "2.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a548d2beca6773b1c244554d36fcf8548a8a58e74156968211567250e48e49a" +dependencies = [ + "pest", + "pest_generator", +] + +[[package]] +name = "pest_generator" +version = "2.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c93a82e8d145725dcbaf44e5ea887c8a869efdcc28706df2d08c69e17077183" +dependencies = [ + "pest", + "pest_meta", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "pest_meta" +version = "2.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a941429fea7e08bedec25e4f6785b6ffaacc6b755da98df5ef3e7dcf4a124c4f" +dependencies = [ + "once_cell", + "pest", + "sha2", +] + [[package]] name = "phf" version = "0.11.2" @@ -2936,6 +3031,17 @@ dependencies = [ "syn", ] +[[package]] +name = "sha2" +version = "0.10.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "793db75ad2bcafc3ffa7c68b215fee268f537982cd901d132f89c6343f3a3dc8" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -3336,6 +3442,18 @@ version = "2.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6af6ae20167a9ece4bcb41af5b80f8a1f1df981f6391189ce00fd257af04126a" +[[package]] +name = "typenum" +version = "1.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42ff0bf0c66b8238c6f3b578df37d0b7848e55df8577b3f74f92a69acceeb825" + +[[package]] +name = "ucd-trie" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed646292ffc8188ef8ea4d1e0e0150fb15a5c2e12ad9b8fc191ae7a8a7f3c4b9" + [[package]] name = "unic-char-property" version = "0.9.0" diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml index a419239bc4f35..d3d933c41f27f 100644 --- a/crates/ruff_linter/Cargo.toml +++ b/crates/ruff_linter/Cargo.toml @@ -73,7 +73,7 @@ unicode-normalization = { workspace = true } url = { workspace = true } [dev-dependencies] -insta = { workspace = true } +insta = { workspace = true, features = ["filters", "json", "redactions"] } test-case = { workspace = true } # Disable colored output in tests colored = { workspace = true, features = ["no-color"] } diff --git a/crates/ruff_linter/src/message/sarif.rs b/crates/ruff_linter/src/message/sarif.rs index 8a354de12b0e4..a04bb441488e9 100644 --- a/crates/ruff_linter/src/message/sarif.rs +++ b/crates/ruff_linter/src/message/sarif.rs @@ -186,7 +186,6 @@ impl Serialize for SarifResult { #[cfg(test)] mod tests { - use crate::message::tests::{ capture_emitter_output, create_messages, create_syntax_error_messages, }; @@ -213,30 +212,11 @@ mod tests { #[test] fn test_results() { let content = get_output(); - let sarif = serde_json::from_str::(content.as_str()).unwrap(); - let rules = sarif["runs"][0]["tool"]["driver"]["rules"] - .as_array() - .unwrap(); - let results = sarif["runs"][0]["results"].as_array().unwrap(); - assert_eq!(results.len(), 3); - assert_eq!( - results - .iter() - .map(|r| r["message"]["text"].as_str().unwrap()) - .collect::>(), - vec![ - "`os` imported but unused", - "Local variable `x` is assigned to but never used", - "Undefined name `a`", - ] - ); - assert_eq!(rules.len(), 3); - assert_eq!( - rules - .iter() - .map(|r| r["id"].as_str().unwrap()) - .collect::>(), - vec!["F401", "F821", "F841"], - ); + let value = serde_json::from_str::(&content).unwrap(); + + insta::assert_json_snapshot!(value, { + ".runs[0].tool.driver.version" => "[VERSION]", + ".runs[0].results[].locations[].physicalLocation.artifactLocation.uri" => "[URI]", + }); } } diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap new file mode 100644 index 0000000000000..a72d48100cba2 --- /dev/null +++ b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap @@ -0,0 +1,146 @@ +--- +source: crates/ruff_linter/src/message/sarif.rs +expression: value +--- +{ + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "results": [ + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[URI]" + }, + "region": { + "endColumn": 10, + "endLine": 1, + "startColumn": 8, + "startLine": 1 + } + } + } + ], + "message": { + "text": "`os` imported but unused" + }, + "ruleId": "F401" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[URI]" + }, + "region": { + "endColumn": 6, + "endLine": 6, + "startColumn": 5, + "startLine": 6 + } + } + } + ], + "message": { + "text": "Local variable `x` is assigned to but never used" + }, + "ruleId": "F841" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[URI]" + }, + "region": { + "endColumn": 5, + "endLine": 1, + "startColumn": 4, + "startLine": 1 + } + } + } + ], + "message": { + "text": "Undefined name `a`" + }, + "ruleId": "F821" + } + ], + "tool": { + "driver": { + "informationUri": "https://github.com/astral-sh/ruff", + "name": "ruff", + "rules": [ + { + "fullDescription": { + "text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.readthedocs.io/en/latest/source/libraries.html#library-interface-public-and-private-symbols)\n" + }, + "help": { + "text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability" + }, + "helpUri": "https://docs.astral.sh/ruff/rules/unused-import", + "id": "F401", + "properties": { + "id": "F401", + "kind": "Pyflakes", + "name": "unused-import", + "problem.severity": "error" + }, + "shortDescription": { + "text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability" + } + }, + { + "fullDescription": { + "text": "## What it does\nChecks for uses of undefined names.\n\n## Why is this bad?\nAn undefined name is likely to raise `NameError` at runtime.\n\n## Example\n```python\ndef double():\n return n * 2 # raises `NameError` if `n` is undefined when `double` is called\n```\n\nUse instead:\n```python\ndef double(n):\n return n * 2\n```\n\n## Options\n- [`target-version`]: Can be used to configure which symbols Ruff will understand\n as being available in the `builtins` namespace.\n\n## References\n- [Python documentation: Naming and binding](https://docs.python.org/3/reference/executionmodel.html#naming-and-binding)\n" + }, + "help": { + "text": "Undefined name `{name}`. {tip}" + }, + "helpUri": "https://docs.astral.sh/ruff/rules/undefined-name", + "id": "F821", + "properties": { + "id": "F821", + "kind": "Pyflakes", + "name": "undefined-name", + "problem.severity": "error" + }, + "shortDescription": { + "text": "Undefined name `{name}`. {tip}" + } + }, + { + "fullDescription": { + "text": "## What it does\nChecks for the presence of unused variables in function scopes.\n\n## Why is this bad?\nA variable that is defined but not used is likely a mistake, and should\nbe removed to avoid confusion.\n\nIf a variable is intentionally defined-but-not-used, it should be\nprefixed with an underscore, or some other value that adheres to the\n[`lint.dummy-variable-rgx`] pattern.\n\nUnder [preview mode](https://docs.astral.sh/ruff/preview), this rule also\ntriggers on unused unpacked assignments (for example, `x, y = foo()`).\n\n## Example\n```python\ndef foo():\n x = 1\n y = 2\n return x\n```\n\nUse instead:\n```python\ndef foo():\n x = 1\n return x\n```\n\n## Options\n- `lint.dummy-variable-rgx`\n" + }, + "help": { + "text": "Local variable `{name}` is assigned to but never used" + }, + "helpUri": "https://docs.astral.sh/ruff/rules/unused-variable", + "id": "F841", + "properties": { + "id": "F841", + "kind": "Pyflakes", + "name": "unused-variable", + "problem.severity": "error" + }, + "shortDescription": { + "text": "Local variable `{name}` is assigned to but never used" + } + } + ], + "version": "[VERSION]" + } + } + } + ], + "version": "2.1.0" +}