diff --git a/Cargo.lock b/Cargo.lock index 08da4b1b7e..cc6bc4ad45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3267,6 +3267,7 @@ dependencies = [ "tracing", "tracing-subscriber", "ty_static", + "url", "web-time", "which", "zip", @@ -3433,7 +3434,6 @@ dependencies = [ "typed-arena", "unicode-normalization", "unicode-width", - "url", ] [[package]] diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__output_format_preview_sarif.snap b/crates/ruff/tests/cli/snapshots/cli__lint__output_format_preview_sarif.snap index d77d57d182..0fe9287ef5 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__output_format_preview_sarif.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__output_format_preview_sarif.snap @@ -119,6 +119,10 @@ exit_code: 1 "name": "ruff", "rules": [ { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, "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" }, @@ -138,6 +142,10 @@ exit_code: 1 } }, { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, "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## Preview\nWhen [preview] is enabled (and certain simplifying assumptions\nare met), we analyze all import statements for a given module\nwhen determining whether an import is used, rather than simply\nthe last of these statements. This can result in both different and\nmore import statements being marked as unused.\n\nFor example, if a module consists of\n\n```python\nimport a\nimport a.b\n```\n\nthen both statements are marked as unused under [preview], whereas\nonly the second is marked as unused under stable behavior.\n\nAs another example, if a module consists of\n\n```python\nimport a.b\nimport a\n\na.b.foo()\n```\n\nthen a diagnostic will be emitted for the second line under [preview],\nwhereas no diagnostic is emitted under stable behavior.\n\nNote that this behavior is somewhat subjective and is designed\nto conform to the developer's intuition rather than Python's actual\nexecution. To wit, the statement `import a.b` automatically executes\n`import a`, so in some sense `import a` is _always_ redundant\nin the presence of `import a.b`.\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\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\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- `lint.pyflakes.allowed-unused-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.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n\n[preview]: https://docs.astral.sh/ruff/preview/\n" }, diff --git a/crates/ruff/tests/cli/snapshots/cli__lint__output_format_sarif.snap b/crates/ruff/tests/cli/snapshots/cli__lint__output_format_sarif.snap index cbd09bb14a..8453743b6f 100644 --- a/crates/ruff/tests/cli/snapshots/cli__lint__output_format_sarif.snap +++ b/crates/ruff/tests/cli/snapshots/cli__lint__output_format_sarif.snap @@ -118,6 +118,10 @@ exit_code: 1 "name": "ruff", "rules": [ { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, "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## Preview\nWhen [preview] is enabled (and certain simplifying assumptions\nare met), we analyze all import statements for a given module\nwhen determining whether an import is used, rather than simply\nthe last of these statements. This can result in both different and\nmore import statements being marked as unused.\n\nFor example, if a module consists of\n\n```python\nimport a\nimport a.b\n```\n\nthen both statements are marked as unused under [preview], whereas\nonly the second is marked as unused under stable behavior.\n\nAs another example, if a module consists of\n\n```python\nimport a.b\nimport a\n\na.b.foo()\n```\n\nthen a diagnostic will be emitted for the second line under [preview],\nwhereas no diagnostic is emitted under stable behavior.\n\nNote that this behavior is somewhat subjective and is designed\nto conform to the developer's intuition rather than Python's actual\nexecution. To wit, the statement `import a.b` automatically executes\n`import a`, so in some sense `import a` is _always_ redundant\nin the presence of `import a.b`.\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\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\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- `lint.pyflakes.allowed-unused-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.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n\n[preview]: https://docs.astral.sh/ruff/preview/\n" }, @@ -137,6 +141,10 @@ exit_code: 1 } }, { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, "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" }, diff --git a/crates/ruff_db/Cargo.toml b/crates/ruff_db/Cargo.toml index 2b4c8a1ea6..2b2ff249e8 100644 --- a/crates/ruff_db/Cargo.toml +++ b/crates/ruff_db/Cargo.toml @@ -47,6 +47,7 @@ supports-hyperlinks = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, optional = true } +url = { workspace = true, optional = true } which = { workspace = true, optional = true } zip = { workspace = true } @@ -57,7 +58,7 @@ etcetera = { workspace = true, optional = true } web-time = { workspace = true, default-features = false } [dev-dependencies] -insta = { workspace = true, features = ["filters"] } +insta = { workspace = true, features = ["filters", "json", "redactions"] } tempfile = { workspace = true } [features] @@ -68,6 +69,7 @@ serde = [ "camino/serde1", "dep:serde", "dep:serde_json", + "dep:url", "ruff_diagnostics/serde", ] # Exposes testing utilities. diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 6f7e696ce9..3576305f82 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -504,20 +504,8 @@ impl Diagnostic { ) } - /// Computes the end source location for the message. - /// - /// Returns None if the diagnostic has no primary span, if its file is not a `SourceFile`, - /// or if the span has no range. - pub fn ruff_end_location(&self) -> Option { - Some( - self.ruff_source_file()? - .to_source_code() - .line_column(self.range()?.end()), - ) - } - /// Returns the [`SourceFile`] which the message belongs to. - pub fn ruff_source_file(&self) -> Option<&SourceFile> { + fn ruff_source_file(&self) -> Option<&SourceFile> { self.primary_span_ref()?.as_ruff_file() } @@ -1418,6 +1406,25 @@ pub enum HyperlinkMode { Never, } +/// Descriptive information about a lint rule, independent of any particular diagnostic. +#[derive(Clone, Debug)] +pub struct RuleMetadata { + /// The canonical name of the rule. + pub name: LintName, + /// A short description of what the rule detects. + pub summary: &'static str, + /// The rule's full documentation. + pub description: Cow<'static, str>, + /// The URL of the rule's documentation. + pub help_url: String, + /// The linter that defines the rule, if applicable. + pub linter: Option<&'static str>, + /// The rule's severity before configuration overrides, if one is declared. + pub default_severity: Option, + /// Whether the rule is enabled by default. + pub enabled_by_default: bool, +} + /// Configuration for rendering diagnostics. #[derive(Clone, Debug)] pub struct DisplayDiagnosticConfig { diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 8f77f3af93..518e83a8c7 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -35,6 +35,8 @@ pub use junit::JunitRenderer; pub use pylint::PylintRenderer; #[cfg(feature = "serde")] pub use rdjson::RdjsonRenderer; +#[cfg(feature = "serde")] +pub use sarif::SarifRenderer; mod azure; mod concise; @@ -51,6 +53,8 @@ mod junit; mod pylint; #[cfg(feature = "serde")] mod rdjson; +#[cfg(feature = "serde")] +mod sarif; /// Renders diagnostics in a particular output format. pub trait Renderer { @@ -1046,6 +1050,8 @@ mod tests { use ruff_diagnostics::{Applicability, Edit, Fix}; + #[cfg(feature = "serde")] + use crate::diagnostic::RuleMetadata; use crate::diagnostic::{ Annotation, DiagnosticId, IntoDiagnosticMessage, SecondaryCode, Severity, Span, SubDiagnosticSeverity, @@ -2470,6 +2476,8 @@ watermelon Rdjson, #[cfg(feature = "serde")] Gitlab, + #[cfg(feature = "serde")] + Sarif, #[cfg(feature = "junit")] Junit, } @@ -2689,6 +2697,29 @@ watermelon DiagnosticFormat::Gitlab => { GitlabRenderer::new(&self.db, &self.config).render(f, diagnostics) } + #[cfg(feature = "serde")] + DiagnosticFormat::Sarif => { + let metadata = diagnostics + .iter() + .filter_map(|diagnostic| diagnostic.id().as_lint()) + .map(|name| { + ( + name, + RuleMetadata { + name, + summary: name.as_str(), + description: format!("Documentation for `{name}`.").into(), + help_url: format!("https://ty.dev/rules#{name}"), + linter: Some("example"), + default_severity: Some(Severity::Error), + enabled_by_default: true, + }, + ) + }) + .collect(); + SarifRenderer::new(&self.db, &self.config, metadata, "1.0.0") + .render(f, diagnostics) + } #[cfg(feature = "junit")] DiagnosticFormat::Junit => { JunitRenderer::new(&self.db, &self.config).render(f, diagnostics) diff --git a/crates/ruff_linter/src/message/sarif.rs b/crates/ruff_db/src/diagnostic/render/sarif.rs similarity index 59% rename from crates/ruff_linter/src/message/sarif.rs rename to crates/ruff_db/src/diagnostic/render/sarif.rs index 9ea54b8cc9..9fff659c38 100644 --- a/crates/ruff_linter/src/message/sarif.rs +++ b/crates/ruff_db/src/diagnostic/render/sarif.rs @@ -1,64 +1,71 @@ use std::borrow::Cow; -use std::collections::HashSet; -use std::io::Write; +use std::collections::HashMap; -use anyhow::Result; -use log::warn; +use rustc_hash::FxHashMap; use serde::{Serialize, Serializer}; -use ruff_db::diagnostic::{Diagnostic, DisplayDiagnosticConfig, SecondaryCode, Severity}; -use ruff_source_file::{OneIndexed, SourceFile}; +use ruff_source_file::{OneIndexed, SourceCode}; use ruff_text_size::{Ranged, TextRange}; -use crate::VERSION; -use crate::codes::Rule; -use crate::fs::normalize_path; -use crate::message::{Emitter, EmitterContext}; -use crate::registry::{Linter, RuleNamespace}; +use crate::diagnostic::{ + Diagnostic, DisplayDiagnosticConfig, LintName, RuleMetadata, SecondaryCode, Severity, +}; -/// An emitter for producing SARIF 2.1.0-compliant JSON output. +use super::{FileResolver, Renderer}; + +/// A renderer for producing SARIF 2.1.0-compliant JSON output. /// /// Static Analysis Results Interchange Format (SARIF) is a standard format /// for static analysis results. For full specification, see: /// [SARIF 2.1.0](https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html) -pub(crate) struct SarifEmitter<'a> { +pub struct SarifRenderer<'a> { + resolver: &'a dyn FileResolver, config: &'a DisplayDiagnosticConfig, + metadata: FxHashMap, + version: &'a str, } -impl<'a> SarifEmitter<'a> { - pub(crate) fn new(config: &'a DisplayDiagnosticConfig) -> Self { - Self { config } +impl<'a> SarifRenderer<'a> { + pub fn new( + resolver: &'a dyn FileResolver, + config: &'a DisplayDiagnosticConfig, + metadata: FxHashMap, + version: &'a str, + ) -> Self { + Self { + resolver, + config, + metadata, + version, + } } } -impl Emitter for SarifEmitter<'_> { - fn emit( - &mut self, - writer: &mut dyn Write, - diagnostics: &[Diagnostic], - _context: &EmitterContext, - ) -> Result<()> { +impl Renderer for SarifRenderer<'_> { + fn render(&self, f: &mut std::fmt::Formatter, diagnostics: &[Diagnostic]) -> std::fmt::Result { let results = diagnostics .iter() - .map(|diagnostic| SarifResult::from_message(diagnostic, self.config)) - .collect::>>()?; + .map(|diagnostic| SarifResult::from_message(diagnostic, self.resolver, self.config)) + .collect::>(); - let unique_rules_and_severities: HashSet<_> = results - .iter() - .filter(|result| result.is_lint_rule) - // Here we implicitly assume that, for a fixed rule, all diagnostics - // have the same severity. If we ever introduce a way for users - // to locally change the severity of a diagnostic we will have to - // refactor in such a way that we know the _configured_ severity at - // this point (rather than just the severities of each diagnostic). - .map(|result| { - let code = result.rule_id.as_str(); - (code, result.level) - }) - .collect(); - let mut rules: Vec = unique_rules_and_severities + let mut unique_rules = HashMap::new(); + for result in &results { + if let Some(name) = result.lint_name { + // A rule's severity can differ between files. Keep one descriptor per rule, + // using its highest reported severity for GitHub's `problem.severity`; + // each result retains its own level. + unique_rules + .entry((name, result.rule_id.as_str())) + .and_modify(|level: &mut SarifLevel| *level = (*level).max(result.level)) + .or_insert(result.level); + } + } + let mut rules: Vec = unique_rules .into_iter() - .map(SarifRule::from) + .filter_map(|((name, code), level)| { + let metadata = self.metadata.get(&name)?; + Some(SarifRule::new(code, level, metadata)) + }) .collect(); rules.sort_by(|a, b| { a.properties @@ -72,17 +79,19 @@ impl Emitter for SarifEmitter<'_> { runs: [SarifRun { tool: SarifTool { driver: SarifDriver { - name: "ruff", - information_uri: "https://github.com/astral-sh/ruff", + name: self.config.program, + information_uri: format!( + "https://github.com/astral-sh/{}", + self.config.program, + ), rules, - version: VERSION.to_string(), + version: self.version, }, }, results: &results, }], }; - serde_json::to_writer_pretty(writer, &output)?; - Ok(()) + write!(f, "{:#}", serde_json::json!(output)) } } @@ -108,63 +117,65 @@ struct SarifTool<'a> { #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct SarifDriver<'a> { - information_uri: &'static str, + information_uri: String, name: &'static str, rules: Vec>, - version: String, + version: &'a str, } #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] struct SarifRule<'a> { + default_configuration: SarifReportingConfiguration, full_description: SarifMessage<'a>, help: SarifMessage<'a>, - help_uri: String, + help_uri: &'a str, id: &'a str, properties: SarifProperties<'a>, short_description: SarifMessage<'a>, } -impl<'a> From<(&'a str, SarifLevel)> for SarifRule<'a> { - fn from(code_and_level: (&'a str, SarifLevel)) -> Self { - let (code, level) = code_and_level; - // This is a manual re-implementation of Rule::from_code, but we also want the Linter. This - // avoids calling Linter::parse_code twice. - let (kind, rule) = match Linter::parse_code(code) { - Some((linter, suffix)) => { - let rule = linter - .all_rules() - .find(|rule| rule.noqa_code().is_some_and(|code| code.suffix() == suffix)) - .expect("Expected a valid noqa code corresponding to a rule"); - (Some(linter.name()), rule) - } - None => { - let rule = Rule::from_name(code).expect("expected a valid rule name"); - (None, rule) - } - }; +impl<'a> SarifRule<'a> { + fn new(code: &'a str, level: SarifLevel, metadata: &'a RuleMetadata) -> Self { Self { id: code, + default_configuration: SarifReportingConfiguration { + enabled: metadata.enabled_by_default, + level: metadata.default_severity.map(SarifLevel::from), + }, short_description: SarifMessage { - text: rule.message_formats()[0].into(), + text: Cow::Borrowed(metadata.summary), }, full_description: SarifMessage { - text: rule.explanation().into(), + text: Cow::Borrowed(&metadata.description), }, help: SarifMessage { - text: rule.message_formats()[0].into(), + text: Cow::Borrowed(metadata.summary), }, - help_uri: rule.url(), + help_uri: &metadata.help_url, properties: SarifProperties { id: code, - kind, - name: rule.into(), + // Preserve the existing SARIF output when preview prefers rule names: + // omit the linter even if the rule also has a code. + kind: if code == metadata.name.as_str() { + None + } else { + metadata.linter + }, + name: metadata.name.as_str(), problem_severity: level, }, } } } +#[derive(Debug, Clone, Serialize)] +struct SarifReportingConfiguration { + enabled: bool, + #[serde(skip_serializing_if = "Option::is_none")] + level: Option, +} + #[derive(Debug)] enum RuleCode<'a> { SecondaryCode(&'a SecondaryCode), @@ -216,7 +227,7 @@ struct SarifResult<'a> { message: SarifMessage<'a>, rule_id: RuleCode<'a>, #[serde(skip)] - is_lint_rule: bool, + lint_name: Option, } #[derive(Clone, Debug, Serialize)] @@ -278,7 +289,7 @@ struct InsertedContent { text: String, } -#[derive(Debug, Serialize, Clone, Copy)] +#[derive(Debug, Default, Serialize, Clone, Copy)] #[serde(rename_all = "camelCase")] struct SarifRegion { end_column: OneIndexed, @@ -315,8 +326,7 @@ impl From for SarifLevel { } impl<'a> SarifResult<'a> { - fn range_to_sarif_region(source_file: &SourceFile, range: TextRange) -> SarifRegion { - let source_code = source_file.to_source_code(); + fn range_to_sarif_region(source_code: &SourceCode, range: TextRange) -> SarifRegion { let start_location = source_code.line_column(range.start()); let end_location = source_code.line_column(range.end()); @@ -328,23 +338,9 @@ impl<'a> SarifResult<'a> { } } - fn fix(diagnostic: &'a Diagnostic, uri: &str) -> Option { + fn fix(diagnostic: &'a Diagnostic, uri: &str, source_code: &SourceCode) -> Option { let fix = diagnostic.fix()?; - let Some(source_file) = diagnostic.ruff_source_file() else { - debug_assert!( - false, - "Omitting the fix for diagnostic with id `{}` because the source file is missing. This is a bug in Ruff, please report an issue.", - diagnostic.id() - ); - - warn!( - "Omitting the fix for diagnostic with id `{}` because the source file is missing. This is a bug in Ruff, please report an issue.", - diagnostic.id() - ); - return None; - }; - let fix_description = diagnostic .first_help_text() .map(std::string::ToString::to_string); @@ -354,7 +350,7 @@ impl<'a> SarifResult<'a> { .iter() .map(|edit| { let range = edit.range(); - let deleted_region = Self::range_to_sarif_region(source_file, range); + let deleted_region = Self::range_to_sarif_region(source_code, range); SarifReplacement { deleted_region, inserted_content: edit.content().map(|content| InsertedContent { @@ -379,63 +375,65 @@ impl<'a> SarifResult<'a> { }) } - #[allow(clippy::unnecessary_wraps)] - fn uri(diagnostic: &Diagnostic) -> Result { - let path = normalize_path(&*diagnostic.expect_ruff_filename()); + fn uri(path: &str) -> String { cfg_select! { - target_arch = "wasm32" => Ok(format!("file://{}", path.display())), - _ => url::Url::from_file_path(&path) - .map_err(|()| anyhow::anyhow!("Failed to convert path to URL: {}", path.display())) - .map(|url| url.to_string()), + target_arch = "wasm32" => format!("file://{path}"), + _ => url::Url::from_file_path(path) + .map_or_else(|()| format!("file://{path}"), |url| url.to_string()), } } fn from_message( diagnostic: &'a Diagnostic, + resolver: &dyn FileResolver, config: &'a DisplayDiagnosticConfig, - ) -> Result { - let start_location = diagnostic.ruff_start_location().unwrap_or_default(); - let end_location = diagnostic.ruff_end_location().unwrap_or_default(); - let region = SarifRegion { - start_line: start_location.line, - start_column: start_location.column, - end_line: end_location.line, - end_column: end_location.column, - }; + ) -> Self { + let (locations, fixes) = diagnostic + .primary_span() + .map(|span| { + let file = span.file(); + let uri = Self::uri(file.path(resolver)); + let source = file.diagnostic_source(resolver); + let source_code = source.as_source_code(); + let region = span + .range() + .map(|range| Self::range_to_sarif_region(&source_code, range)) + .unwrap_or_default(); + let fixes = Self::fix(diagnostic, &uri, &source_code) + .into_iter() + .collect(); + let locations = vec![SarifLocation { + physical_location: SarifPhysicalLocation { + artifact_location: SarifArtifactLocation { uri }, + region, + }, + }]; + (locations, fixes) + }) + .unwrap_or_default(); - let uri = Self::uri(diagnostic)?; - - Ok(Self { + Self { rule_id: RuleCode::from_diagnostic(diagnostic, config), level: SarifLevel::from(diagnostic.severity()), message: SarifMessage { text: diagnostic.concise_message().to_str(), }, - fixes: Self::fix(diagnostic, &uri).into_iter().collect(), - locations: vec![SarifLocation { - physical_location: SarifPhysicalLocation { - artifact_location: SarifArtifactLocation { uri }, - region, - }, - }], - is_lint_rule: diagnostic.id().is_lint(), - }) + fixes, + locations, + lint_name: diagnostic.id().as_lint(), + } } } #[cfg(test)] mod tests { - use ruff_db::diagnostic::DisplayDiagnosticConfig; - - use crate::message::SarifEmitter; - use crate::message::tests::{ - capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics, + use crate::diagnostic::render::tests::{ + DiagnosticFormat, create_diagnostics, create_syntax_error_diagnostics, }; fn get_output() -> String { - let config = DisplayDiagnosticConfig::new("ruff"); - let mut emitter = SarifEmitter::new(&config); - capture_emitter_output(&mut emitter, &create_diagnostics()) + let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Sarif); + env.render_diagnostics(&diagnostics) } #[test] @@ -446,9 +444,8 @@ mod tests { #[test] fn valid_syntax_error_json() { - let config = DisplayDiagnosticConfig::new("ruff"); - let mut emitter = SarifEmitter::new(&config); - let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics()); + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Sarif); + let content = env.render_diagnostics(&diagnostics); serde_json::from_str::(&content).unwrap(); } diff --git a/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__sarif__tests__results.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__sarif__tests__results.snap new file mode 100644 index 0000000000..d76450690e --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__sarif__tests__results.snap @@ -0,0 +1,228 @@ +--- +source: crates/ruff_db/src/diagnostic/render/sarif.rs +expression: value +--- +{ + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "results": [ + { + "fixes": [ + { + "artifactChanges": [ + { + "artifactLocation": { + "uri": "[URI]" + }, + "replacements": [ + { + "deletedRegion": { + "endColumn": 1, + "endLine": 2, + "startColumn": 1, + "startLine": 1 + } + } + ] + } + ], + "description": { + "text": "Remove unused import: `os`" + } + } + ], + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[URI]" + }, + "region": { + "endColumn": 10, + "endLine": 1, + "startColumn": 8, + "startLine": 1 + } + } + } + ], + "message": { + "text": "`os` imported but unused" + }, + "ruleId": "F401" + }, + { + "fixes": [ + { + "artifactChanges": [ + { + "artifactLocation": { + "uri": "[URI]" + }, + "replacements": [ + { + "deletedRegion": { + "endColumn": 10, + "endLine": 6, + "startColumn": 5, + "startLine": 6 + } + } + ] + } + ], + "description": { + "text": "Remove assignment to unused variable `x`" + } + } + ], + "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" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "[URI]" + }, + "region": { + "endColumn": 26, + "endLine": 12, + "startColumn": 16, + "startLine": 12 + } + } + } + ], + "message": { + "text": "Undefined name `fibonaccii`" + }, + "ruleId": "F821" + } + ], + "tool": { + "driver": { + "informationUri": "https://github.com/astral-sh/ty", + "name": "ty", + "rules": [ + { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, + "fullDescription": { + "text": "Documentation for `unused-import`." + }, + "help": { + "text": "unused-import" + }, + "helpUri": "https://ty.dev/rules#unused-import", + "id": "F401", + "properties": { + "id": "F401", + "kind": "example", + "name": "unused-import", + "problem.severity": "error" + }, + "shortDescription": { + "text": "unused-import" + } + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, + "fullDescription": { + "text": "Documentation for `undefined-name`." + }, + "help": { + "text": "undefined-name" + }, + "helpUri": "https://ty.dev/rules#undefined-name", + "id": "F821", + "properties": { + "id": "F821", + "kind": "example", + "name": "undefined-name", + "problem.severity": "error" + }, + "shortDescription": { + "text": "undefined-name" + } + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, + "fullDescription": { + "text": "Documentation for `unused-variable`." + }, + "help": { + "text": "unused-variable" + }, + "helpUri": "https://ty.dev/rules#unused-variable", + "id": "F841", + "properties": { + "id": "F841", + "kind": "example", + "name": "unused-variable", + "problem.severity": "error" + }, + "shortDescription": { + "text": "unused-variable" + } + } + ], + "version": "[VERSION]" + } + } + } + ], + "version": "2.1.0" +} diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml index d9e23a30ba..2c62cd4417 100644 --- a/crates/ruff_linter/Cargo.toml +++ b/crates/ruff_linter/Cargo.toml @@ -67,7 +67,6 @@ toml = { workspace = true } typed-arena = { workspace = true } unicode-normalization = { workspace = true } unicode-width = { workspace = true } -url = { workspace = true } [dev-dependencies] # Disable colored output in tests diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 36355f6a28..f1815b53e0 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -5,14 +5,16 @@ use std::path::Path; use ruff_db::panic::PanicError; use rustc_hash::FxHashMap; +use strum::IntoEnumIterator; use ruff_db::diagnostic::render::{ AzureRenderer, ConciseRenderer, FullRenderer, GithubRenderer, GitlabRenderer, JsonLinesRenderer, JsonRenderer, JunitRenderer, PylintRenderer, RdjsonRenderer, Renderer, + SarifRenderer, }; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticId, DisplayDiagnosticConfig, FileResolver, Input, LintName, - SecondaryCode, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, UnifiedFile, + RuleMetadata, SecondaryCode, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, UnifiedFile, }; use ruff_db::files::File; @@ -20,14 +22,14 @@ pub(crate) use grouped::GroupedEmitter; use ruff_notebook::NotebookIndex; use ruff_source_file::{SourceFile, SourceFileBuilder}; use ruff_text_size::{TextRange, TextSize}; -pub(crate) use sarif::SarifEmitter; use crate::Fix; -use crate::registry::Rule; +use crate::registry::{Linter, Rule, RuleNamespace, RuleSet}; +use crate::rule_selector::PreviewOptions; +use crate::settings::DEFAULT_SELECTORS; use crate::settings::types::OutputFormat; mod grouped; -mod sarif; /// Create a `Diagnostic` from a panic. pub fn create_panic_diagnostic(error: &PanicError, path: Option<&Path>) -> Diagnostic { @@ -168,6 +170,36 @@ impl FileResolver for EmitterContext<'_> { } } +fn rule_metadata() -> FxHashMap { + // Describe the defaults before configuration overrides such as preview. + let default_rules = DEFAULT_SELECTORS + .iter() + .flat_map(|selector| selector.rules(&PreviewOptions::default())) + .collect::(); + + Rule::iter() + .map(|rule| { + let name = LintName::of(rule.into()); + let linter = rule.noqa_code().and_then(|code| { + Linter::parse_code(&code.to_string()).map(|(linter, _)| linter.name()) + }); + + ( + name, + RuleMetadata { + name, + summary: rule.message_formats()[0], + description: rule.explanation().into(), + help_url: rule.url(), + linter, + default_severity: Some(Severity::Error), + enabled_by_default: default_rules.contains(rule), + }, + ) + }) + .collect() +} + /// Display format for [`Diagnostic`]s. /// /// The emitter serializes a slice of [`Diagnostic`]s and writes them to a [`Write`]. @@ -223,9 +255,8 @@ pub fn render_diagnostics( .with_prefer_rule_codes(config.is_prefer_rule_codes_enabled()) .emit(writer, diagnostics, context) .map_err(std::io::Error::other), - OutputFormat::Sarif => SarifEmitter::new(config) - .emit(writer, diagnostics, context) - .map_err(std::io::Error::other), + OutputFormat::Sarif => SarifRenderer::new(context, config, rule_metadata(), crate::VERSION) + .write(writer, diagnostics), } } 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 deleted file mode 100644 index 1da0aa01d8..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap +++ /dev/null @@ -1,194 +0,0 @@ ---- -source: crates/ruff_linter/src/message/sarif.rs -expression: value ---- -{ - "$schema": "https://json.schemastore.org/sarif-2.1.0.json", - "runs": [ - { - "results": [ - { - "fixes": [ - { - "artifactChanges": [ - { - "artifactLocation": { - "uri": "[URI]" - }, - "replacements": [ - { - "deletedRegion": { - "endColumn": 1, - "endLine": 2, - "startColumn": 1, - "startLine": 1 - } - } - ] - } - ], - "description": { - "text": "Remove unused import: `os`" - } - } - ], - "level": "error", - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "[URI]" - }, - "region": { - "endColumn": 10, - "endLine": 1, - "startColumn": 8, - "startLine": 1 - } - } - } - ], - "message": { - "text": "`os` imported but unused" - }, - "ruleId": "F401" - }, - { - "fixes": [ - { - "artifactChanges": [ - { - "artifactLocation": { - "uri": "[URI]" - }, - "replacements": [ - { - "deletedRegion": { - "endColumn": 10, - "endLine": 6, - "startColumn": 5, - "startLine": 6 - } - } - ] - } - ], - "description": { - "text": "Remove assignment to unused variable `x`" - } - } - ], - "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## Preview\nWhen [preview] is enabled (and certain simplifying assumptions\nare met), we analyze all import statements for a given module\nwhen determining whether an import is used, rather than simply\nthe last of these statements. This can result in both different and\nmore import statements being marked as unused.\n\nFor example, if a module consists of\n\n```python\nimport a\nimport a.b\n```\n\nthen both statements are marked as unused under [preview], whereas\nonly the second is marked as unused under stable behavior.\n\nAs another example, if a module consists of\n\n```python\nimport a.b\nimport a\n\na.b.foo()\n```\n\nthen a diagnostic will be emitted for the second line under [preview],\nwhereas no diagnostic is emitted under stable behavior.\n\nNote that this behavior is somewhat subjective and is designed\nto conform to the developer's intuition rather than Python's actual\nexecution. To wit, the statement `import a.b` automatically executes\n`import a`, so in some sense `import a` is _always_ redundant\nin the presence of `import a.b`.\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\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\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- `lint.pyflakes.allowed-unused-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.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n\n[preview]: https://docs.astral.sh/ruff/preview/\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\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## Fix safety\n\nThis rule's fix is marked as unsafe because removing an unused variable assignment may\ndelete comments that are attached to the assignment.\n\n## See also\n\nThis rule does not apply to bindings in unpacked assignments (e.g. `x, y = 1, 2`). See\n[`unused-unpacked-variable`][RUF059] for this case.\n\n## Options\n- `lint.dummy-variable-rgx`\n\n[RUF059]: https://docs.astral.sh/ruff/rules/unused-unpacked-variable/\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" -} diff --git a/crates/ty/docs/cli.md b/crates/ty/docs/cli.md index c8306663a3..ee0300ae54 100644 --- a/crates/ty/docs/cli.md +++ b/crates/ty/docs/cli.md @@ -77,6 +77,7 @@ over all configuration files.

  • gitlab: Print diagnostics in the JSON format expected by GitLab Code Quality reports
  • github: Print diagnostics in the format used by GitHub Actions workflow error annotations
  • junit: Print diagnostics as a JUnit-style XML report
    • +
  • sarif: Print diagnostics as a SARIF report
    • --project project

    Run the command within the given project directory.

    All pyproject.toml files will be discovered by walking up the directory tree from the given project directory, as will the project's virtual environment (.venv) unless the venv-path option is set.

    Other command-line arguments (such as relative paths) will be resolved relative to the current working directory.

    diff --git a/crates/ty/docs/configuration.md b/crates/ty/docs/configuration.md index 380df79c7a..5468150ded 100644 --- a/crates/ty/docs/configuration.md +++ b/crates/ty/docs/configuration.md @@ -1308,7 +1308,7 @@ Defaults to `full`. **Default value**: `full` -**Type**: `full | concise | github | gitlab | junit` +**Type**: `full | concise | github | gitlab | junit | sarif` **Example usage**: diff --git a/crates/ty/src/args.rs b/crates/ty/src/args.rs index 56972b3af5..27a2a8a514 100644 --- a/crates/ty/src/args.rs +++ b/crates/ty/src/args.rs @@ -449,6 +449,9 @@ pub enum OutputFormat { /// Print diagnostics as a JUnit-style XML report. #[value(name = "junit")] Junit, + /// Print diagnostics as a SARIF report. + #[value(name = "sarif")] + Sarif, } impl From for ty_project::metadata::options::OutputFormat { @@ -459,6 +462,7 @@ impl From for ty_project::metadata::options::OutputFormat { OutputFormat::Gitlab => Self::Gitlab, OutputFormat::Github => Self::Github, OutputFormat::Junit => Self::Junit, + OutputFormat::Sarif => Self::Sarif, } } } diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index 6045138370..09a8c8b9bb 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -767,6 +767,248 @@ fn github_diagnostics() -> anyhow::Result<()> { Ok(()) } +#[test] +fn sarif_diagnostics() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ( + "ty.toml", + r#" + [rules] + division-by-zero = "warn" + unused-ignore-comment = "error" + + [[overrides]] + include = ["a.py"] + + [overrides.rules] + unresolved-reference = "warn" + "#, + ), + ("a.py", "print(x)"), + ( + "b.py", + r#" + print(x) + 1 / 0 # ty: ignore[unresolved-reference] + "#, + ), + ])? + .with_filter( + r#"("fullDescription": \{\s*"text": ")[^\n]*(")"#, + "$1[DESCRIPTION]$2", + ) + .with_filter(r#"(?m)^(\s{10}"version": ")[^"]+"#, "$1[VERSION]") + .with_filter(r#"file://[^"]*/project/"#, "file:///"); + + assert_cmd_snapshot!(case.command().arg("--output-format=sarif"), @r#" + success: false + exit_code: 1 + ----- stdout ----- + { + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "results": [ + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "file:///a.py" + }, + "region": { + "endColumn": 8, + "endLine": 1, + "startColumn": 7, + "startLine": 1 + } + } + } + ], + "message": { + "text": "Name `x` used when not defined" + }, + "ruleId": "unresolved-reference" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "file:///b.py" + }, + "region": { + "endColumn": 8, + "endLine": 2, + "startColumn": 7, + "startLine": 2 + } + } + } + ], + "message": { + "text": "Name `x` used when not defined" + }, + "ruleId": "unresolved-reference" + }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "file:///b.py" + }, + "region": { + "endColumn": 6, + "endLine": 3, + "startColumn": 1, + "startLine": 3 + } + } + } + ], + "message": { + "text": "Cannot divide object of type `Literal[1]` by zero" + }, + "ruleId": "division-by-zero" + }, + { + "fixes": [ + { + "artifactChanges": [ + { + "artifactLocation": { + "uri": "file:///b.py" + }, + "replacements": [ + { + "deletedRegion": { + "endColumn": 42, + "endLine": 3, + "startColumn": 6, + "startLine": 3 + } + } + ] + } + ], + "description": { + "text": "Remove the unused suppression comment" + } + } + ], + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "file:///b.py" + }, + "region": { + "endColumn": 42, + "endLine": 3, + "startColumn": 8, + "startLine": 3 + } + } + } + ], + "message": { + "text": "Unused `ty: ignore` directive" + }, + "ruleId": "unused-ignore-comment" + } + ], + "tool": { + "driver": { + "informationUri": "https://github.com/astral-sh/ty", + "name": "ty", + "rules": [ + { + "defaultConfiguration": { + "enabled": false + }, + "fullDescription": { + "text": "[DESCRIPTION]" + }, + "help": { + "text": "detects division by zero" + }, + "helpUri": "https://ty.dev/rules#division-by-zero", + "id": "division-by-zero", + "properties": { + "id": "division-by-zero", + "kind": null, + "name": "division-by-zero", + "problem.severity": "warning" + }, + "shortDescription": { + "text": "detects division by zero" + } + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "error" + }, + "fullDescription": { + "text": "[DESCRIPTION]" + }, + "help": { + "text": "detects references to names that are not defined" + }, + "helpUri": "https://ty.dev/rules#unresolved-reference", + "id": "unresolved-reference", + "properties": { + "id": "unresolved-reference", + "kind": null, + "name": "unresolved-reference", + "problem.severity": "error" + }, + "shortDescription": { + "text": "detects references to names that are not defined" + } + }, + { + "defaultConfiguration": { + "enabled": true, + "level": "warning" + }, + "fullDescription": { + "text": "[DESCRIPTION]" + }, + "help": { + "text": "detects unused `ty: ignore` comments" + }, + "helpUri": "https://ty.dev/rules#unused-ignore-comment", + "id": "unused-ignore-comment", + "properties": { + "id": "unused-ignore-comment", + "kind": null, + "name": "unused-ignore-comment", + "problem.severity": "error" + }, + "shortDescription": { + "text": "detects unused `ty: ignore` comments" + } + } + ], + "version": "[VERSION]" + } + } + } + ], + "version": "2.1.0" + } + ----- stderr ----- + "#); + + Ok(()) +} + /// This tests the diagnostic format for revealed type. /// /// This test was introduced because changes were made to diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 5ebb93e593..7f2104e15a 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -12,7 +12,7 @@ use ruff_db::RustDoc; #[cfg(feature = "junit")] use ruff_db::diagnostic::render::JunitRenderer; use ruff_db::diagnostic::render::{ - ConciseRenderer, FileResolver, FullRenderer, GithubRenderer, GitlabRenderer, Renderer, + ConciseRenderer, FullRenderer, GithubRenderer, GitlabRenderer, Renderer, SarifRenderer, }; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticId, DisplayDiagnosticConfig, Severity, Span, SubDiagnostic, @@ -1467,6 +1467,10 @@ pub enum OutputFormat { /// Print diagnostics as a JUnit-style XML report. #[cfg(feature = "junit")] Junit, + /// Print diagnostics as a [SARIF] report. + /// + /// [SARIF]: https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html + Sarif, } impl OutputFormat { @@ -1482,17 +1486,25 @@ impl OutputFormat { /// Display diagnostics in this output format. pub fn display<'a>( self, - resolver: &'a dyn FileResolver, + db: &'a dyn Db, config: &'a DisplayDiagnosticConfig, diagnostics: &'a [Diagnostic], ) -> impl Display + use<'a> { + let resolver: &dyn ruff_db::Db = db; fmt::from_fn(move |f| match self { - Self::Full => FullRenderer::new(resolver, config).render(f, diagnostics), - Self::Concise => ConciseRenderer::new(resolver, config).render(f, diagnostics), - Self::Gitlab => GitlabRenderer::new(resolver, config).render(f, diagnostics), - Self::Github => GithubRenderer::new(resolver, config).render(f, diagnostics), + Self::Full => FullRenderer::new(&resolver, config).render(f, diagnostics), + Self::Concise => ConciseRenderer::new(&resolver, config).render(f, diagnostics), + Self::Gitlab => GitlabRenderer::new(&resolver, config).render(f, diagnostics), + Self::Github => GithubRenderer::new(&resolver, config).render(f, diagnostics), #[cfg(feature = "junit")] - Self::Junit => JunitRenderer::new(resolver, config).render(f, diagnostics), + Self::Junit => JunitRenderer::new(&resolver, config).render(f, diagnostics), + Self::Sarif => SarifRenderer::new( + &resolver, + config, + db.lint_registry().rule_metadata(), + ruff_db::program_version().unwrap_or("Unknown"), + ) + .render(f, diagnostics), }) } } @@ -1528,7 +1540,7 @@ pub struct TerminalOptions { #[serde(skip_serializing_if = "Option::is_none")] #[option( default = r#"full"#, - value_type = "full | concise | github | gitlab | junit", + value_type = "full | concise | github | gitlab | junit | sarif", example = r#" output-format = "concise" "# @@ -2282,19 +2294,14 @@ pub struct ToSettingsError { impl ToSettingsError { pub(crate) fn pretty<'a>(&'a self, db: &'a dyn Db) -> impl fmt::Display + use<'a> { - let db: &dyn ruff_db::Db = db; - fmt::from_fn(move |f| { let display_config = DisplayDiagnosticConfig::new("ty").color(self.color); write!( f, "{}", - self.output_format.display( - &db, - &display_config, - &[self.diagnostic.to_diagnostic()] - ) + self.output_format + .display(db, &display_config, &[self.diagnostic.to_diagnostic()]) ) }) } diff --git a/crates/ty_python_semantic/src/lint.rs b/crates/ty_python_semantic/src/lint.rs index 2dbf94d706..c2044c9d19 100644 --- a/crates/ty_python_semantic/src/lint.rs +++ b/crates/ty_python_semantic/src/lint.rs @@ -1,7 +1,7 @@ use crate::diagnostic::did_you_mean; use core::fmt; use itertools::Itertools; -use ruff_db::diagnostic::{DiagnosticId, LintName, Severity}; +use ruff_db::diagnostic::{DiagnosticId, LintName, RuleMetadata, Severity}; use rustc_hash::FxHashMap; use std::error::Error; use std::fmt::Formatter; @@ -481,6 +481,28 @@ impl LintRegistry { } }) } + + /// Returns the metadata used to render all registered, non-removed lints. + pub fn rule_metadata(&self) -> FxHashMap { + self.lints() + .iter() + .map(|lint| { + let name = lint.name(); + ( + name, + RuleMetadata { + name, + summary: lint.summary, + description: lint.documentation().into(), + help_url: lint.documentation_url(), + linter: None, + default_severity: Severity::try_from(lint.default_level()).ok(), + enabled_by_default: !lint.default_level().is_ignore(), + }, + ) + }) + .collect() + } } #[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)] diff --git a/hawk.toml b/hawk.toml index 24fca67797..57a41bbf62 100644 --- a/hawk.toml +++ b/hawk.toml @@ -671,7 +671,7 @@ level = "expect" reason = "lint level predicates intentionally cover every severity" [[override]] -lint = "hawk::dead_public" +lint = "hawk::unnecessary_public" crate = "ty_python_semantic" item = "lint::Level::is_ignore" kind = "inherent_method" diff --git a/ty.schema.json b/ty.schema.json index 08946327b0..68b2b98cfe 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -252,6 +252,11 @@ "description": "Print diagnostics as a JUnit-style XML report.", "type": "string", "const": "junit" + }, + { + "description": "Print diagnostics as a [SARIF] report.\n\n[SARIF]: https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html", + "type": "string", + "const": "sarif" } ] },