Skip to content

Commit 1ce6571

Browse files
authored
Move GitLab output rendering to ruff_db (#20117)
## Summary This PR is a first step toward adding a GitLab output format to ty. It converts the `GitlabEmitter` from `ruff_linter` to a `GitlabRenderer` in `ruff_db` and updates its implementation to handle non-Ruff files and diagnostics without primary spans. I tried to break up the changes here so that they're easy to review commit-by-commit, or at least in groups of commits: - [preparatory changes in-place in `ruff_linter` and a `ruff_db` skeleton](https://github.com/astral-sh/ruff/pull/20117/files/0761b73a615537fe75fc54ba8f7d5f52c28b2a2a) - [moving the code over with no implementation changes mixed in](https://github.com/astral-sh/ruff/pull/20117/files/0761b73a615537fe75fc54ba8f7d5f52c28b2a2a..8f909ea0bb0892107824b311f3982eb3cb1833ed) - [tidying up the code now in `ruff_db`](https://github.com/astral-sh/ruff/pull/20117/files/9f047c4f9f1c826b963f0fa82ca2411d01d6ec83..e5e217fcd611ab24f516b2af4fa21e25eadc5645) This wasn't strictly necessary, but I also added some `Serialize` structs instead of calling `json!` to make it a little clearer that we weren't modifying the schema (e4c4bee). I plan to follow this up with a separate PR exposing this output format in the ty CLI, which should be quite straightforward. ## Test Plan Existing tests, especially the two that show up in the diff as renamed nearly without changes
1 parent d9aaacd commit 1ce6571

File tree

13 files changed

+277
-244
lines changed

13 files changed

+277
-244
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ruff/src/printer.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ use ruff_db::diagnostic::{
1515
use ruff_linter::fs::relativize_path;
1616
use ruff_linter::logging::LogLevel;
1717
use ruff_linter::message::{
18-
Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter, SarifEmitter,
19-
TextEmitter,
18+
Emitter, EmitterContext, GithubEmitter, GroupedEmitter, SarifEmitter, TextEmitter,
2019
};
2120
use ruff_linter::notify_user;
2221
use ruff_linter::settings::flags::{self};
@@ -296,7 +295,11 @@ impl Printer {
296295
GithubEmitter.emit(writer, &diagnostics.inner, &context)?;
297296
}
298297
OutputFormat::Gitlab => {
299-
GitlabEmitter::default().emit(writer, &diagnostics.inner, &context)?;
298+
let config = DisplayDiagnosticConfig::default()
299+
.format(DiagnosticFormat::Gitlab)
300+
.preview(preview);
301+
let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner);
302+
write!(writer, "{value}")?;
300303
}
301304
OutputFormat::Pylint => {
302305
let config = DisplayDiagnosticConfig::default()

crates/ruff/tests/snapshots/lint__output_format_gitlab.snap

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,59 +20,59 @@ exit_code: 1
2020
{
2121
"check_name": "F401",
2222
"description": "F401: `os` imported but unused",
23+
"severity": "major",
2324
"fingerprint": "4dbad37161e65c72",
2425
"location": {
2526
"path": "input.py",
2627
"positions": {
2728
"begin": {
28-
"column": 8,
29-
"line": 1
29+
"line": 1,
30+
"column": 8
3031
},
3132
"end": {
32-
"column": 10,
33-
"line": 1
33+
"line": 1,
34+
"column": 10
3435
}
3536
}
36-
},
37-
"severity": "major"
37+
}
3838
},
3939
{
4040
"check_name": "F821",
4141
"description": "F821: Undefined name `y`",
42+
"severity": "major",
4243
"fingerprint": "7af59862a085230",
4344
"location": {
4445
"path": "input.py",
4546
"positions": {
4647
"begin": {
47-
"column": 5,
48-
"line": 2
48+
"line": 2,
49+
"column": 5
4950
},
5051
"end": {
51-
"column": 6,
52-
"line": 2
52+
"line": 2,
53+
"column": 6
5354
}
5455
}
55-
},
56-
"severity": "major"
56+
}
5757
},
5858
{
5959
"check_name": "invalid-syntax",
6060
"description": "invalid-syntax: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)",
61+
"severity": "major",
6162
"fingerprint": "e558cec859bb66e8",
6263
"location": {
6364
"path": "input.py",
6465
"positions": {
6566
"begin": {
66-
"column": 1,
67-
"line": 3
67+
"line": 3,
68+
"column": 1
6869
},
6970
"end": {
70-
"column": 6,
71-
"line": 3
71+
"line": 3,
72+
"column": 6
7273
}
7374
}
74-
},
75-
"severity": "major"
75+
}
7676
}
7777
]
7878
----- stderr -----

crates/ruff_db/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ glob = { workspace = true }
3434
ignore = { workspace = true, optional = true }
3535
matchit = { workspace = true }
3636
path-slash = { workspace = true }
37+
pathdiff = { workspace = true }
3738
quick-junit = { workspace = true, optional = true }
3839
rustc-hash = { workspace = true }
3940
salsa = { workspace = true }
@@ -53,7 +54,7 @@ web-time = { version = "1.1.0" }
5354
etcetera = { workspace = true, optional = true }
5455

5556
[dev-dependencies]
56-
insta = { workspace = true }
57+
insta = { workspace = true, features = ["filters"] }
5758
tempfile = { workspace = true }
5859

5960
[features]

crates/ruff_db/src/diagnostic/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,11 @@ pub enum DiagnosticFormat {
14351435
/// Print diagnostics in the format expected by JUnit.
14361436
#[cfg(feature = "junit")]
14371437
Junit,
1438+
/// Print diagnostics in the JSON format used by GitLab [Code Quality] reports.
1439+
///
1440+
/// [Code Quality]: https://docs.gitlab.com/ee/ci/testing/code_quality.html#implement-a-custom-tool
1441+
#[cfg(feature = "serde")]
1442+
Gitlab,
14381443
}
14391444

14401445
/// A representation of the kinds of messages inside a diagnostic.

crates/ruff_db/src/diagnostic/render.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ mod azure;
3131
mod concise;
3232
mod full;
3333
#[cfg(feature = "serde")]
34+
mod gitlab;
35+
#[cfg(feature = "serde")]
3436
mod json;
3537
#[cfg(feature = "serde")]
3638
mod json_lines;
@@ -136,6 +138,10 @@ impl std::fmt::Display for DisplayDiagnostics<'_> {
136138
DiagnosticFormat::Junit => {
137139
junit::JunitRenderer::new(self.resolver).render(f, self.diagnostics)?;
138140
}
141+
#[cfg(feature = "serde")]
142+
DiagnosticFormat::Gitlab => {
143+
gitlab::GitlabRenderer::new(self.resolver).render(f, self.diagnostics)?;
144+
}
139145
}
140146

141147
Ok(())
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
use std::{
2+
collections::HashSet,
3+
hash::{DefaultHasher, Hash, Hasher},
4+
path::Path,
5+
};
6+
7+
use ruff_source_file::LineColumn;
8+
use serde::{Serialize, Serializer, ser::SerializeSeq};
9+
10+
use crate::diagnostic::{Diagnostic, Severity};
11+
12+
use super::FileResolver;
13+
14+
pub(super) struct GitlabRenderer<'a> {
15+
resolver: &'a dyn FileResolver,
16+
}
17+
18+
impl<'a> GitlabRenderer<'a> {
19+
pub(super) fn new(resolver: &'a dyn FileResolver) -> Self {
20+
Self { resolver }
21+
}
22+
}
23+
24+
impl GitlabRenderer<'_> {
25+
pub(super) fn render(
26+
&self,
27+
f: &mut std::fmt::Formatter,
28+
diagnostics: &[Diagnostic],
29+
) -> std::fmt::Result {
30+
write!(
31+
f,
32+
"{}",
33+
serde_json::to_string_pretty(&SerializedMessages {
34+
diagnostics,
35+
resolver: self.resolver,
36+
#[expect(
37+
clippy::disallowed_methods,
38+
reason = "We don't have access to a `System` here, \
39+
and this is only intended for use by GitLab CI, \
40+
which runs on a real `System`."
41+
)]
42+
project_dir: std::env::var("CI_PROJECT_DIR").ok().as_deref(),
43+
})
44+
.unwrap()
45+
)
46+
}
47+
}
48+
49+
struct SerializedMessages<'a> {
50+
diagnostics: &'a [Diagnostic],
51+
resolver: &'a dyn FileResolver,
52+
project_dir: Option<&'a str>,
53+
}
54+
55+
impl Serialize for SerializedMessages<'_> {
56+
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
57+
where
58+
S: Serializer,
59+
{
60+
let mut s = serializer.serialize_seq(Some(self.diagnostics.len()))?;
61+
let mut fingerprints = HashSet::<u64>::with_capacity(self.diagnostics.len());
62+
63+
for diagnostic in self.diagnostics {
64+
let location = diagnostic
65+
.primary_span()
66+
.map(|span| {
67+
let file = span.file();
68+
let positions = if self.resolver.is_notebook(file) {
69+
// We can't give a reasonable location for the structured formats,
70+
// so we show one that's clearly a fallback
71+
Default::default()
72+
} else {
73+
let diagnostic_source = file.diagnostic_source(self.resolver);
74+
let source_code = diagnostic_source.as_source_code();
75+
span.range()
76+
.map(|range| Positions {
77+
begin: source_code.line_column(range.start()),
78+
end: source_code.line_column(range.end()),
79+
})
80+
.unwrap_or_default()
81+
};
82+
83+
let path = self.project_dir.as_ref().map_or_else(
84+
|| file.relative_path(self.resolver).display().to_string(),
85+
|project_dir| relativize_path_to(file.path(self.resolver), project_dir),
86+
);
87+
88+
Location { path, positions }
89+
})
90+
.unwrap_or_default();
91+
92+
let mut message_fingerprint = fingerprint(diagnostic, &location.path, 0);
93+
94+
// Make sure that we do not get a fingerprint that is already in use
95+
// by adding in the previously generated one.
96+
while fingerprints.contains(&message_fingerprint) {
97+
message_fingerprint = fingerprint(diagnostic, &location.path, message_fingerprint);
98+
}
99+
fingerprints.insert(message_fingerprint);
100+
101+
let description = diagnostic.body();
102+
let check_name = diagnostic.secondary_code_or_id();
103+
let severity = match diagnostic.severity() {
104+
Severity::Info => "info",
105+
Severity::Warning => "minor",
106+
Severity::Error => "major",
107+
// Another option here is `blocker`
108+
Severity::Fatal => "critical",
109+
};
110+
111+
let value = Message {
112+
check_name,
113+
// GitLab doesn't display the separate `check_name` field in a Code Quality report,
114+
// so prepend it to the description too.
115+
description: format!("{check_name}: {description}"),
116+
severity,
117+
fingerprint: format!("{:x}", message_fingerprint),
118+
location,
119+
};
120+
121+
s.serialize_element(&value)?;
122+
}
123+
124+
s.end()
125+
}
126+
}
127+
128+
#[derive(Serialize)]
129+
struct Message<'a> {
130+
check_name: &'a str,
131+
description: String,
132+
severity: &'static str,
133+
fingerprint: String,
134+
location: Location,
135+
}
136+
137+
/// The place in the source code where the issue was discovered.
138+
///
139+
/// According to the CodeClimate report format [specification] linked from the GitLab [docs], this
140+
/// field is required, so we fall back on a default `path` and position if the diagnostic doesn't
141+
/// have a primary span.
142+
///
143+
/// [specification]: https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#data-types
144+
/// [docs]: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format
145+
#[derive(Default, Serialize)]
146+
struct Location {
147+
path: String,
148+
positions: Positions,
149+
}
150+
151+
#[derive(Default, Serialize)]
152+
struct Positions {
153+
begin: LineColumn,
154+
end: LineColumn,
155+
}
156+
157+
/// Generate a unique fingerprint to identify a violation.
158+
fn fingerprint(diagnostic: &Diagnostic, project_path: &str, salt: u64) -> u64 {
159+
let mut hasher = DefaultHasher::new();
160+
161+
salt.hash(&mut hasher);
162+
diagnostic.name().hash(&mut hasher);
163+
project_path.hash(&mut hasher);
164+
165+
hasher.finish()
166+
}
167+
168+
/// Convert an absolute path to be relative to the specified project root.
169+
fn relativize_path_to<P: AsRef<Path>, R: AsRef<Path>>(path: P, project_root: R) -> String {
170+
format!(
171+
"{}",
172+
pathdiff::diff_paths(&path, project_root)
173+
.expect("Could not diff paths")
174+
.display()
175+
)
176+
}
177+
178+
#[cfg(test)]
179+
mod tests {
180+
use crate::diagnostic::{
181+
DiagnosticFormat,
182+
render::tests::{create_diagnostics, create_syntax_error_diagnostics},
183+
};
184+
185+
const FINGERPRINT_FILTERS: [(&str, &str); 1] = [(
186+
r#""fingerprint": "[a-z0-9]+","#,
187+
r#""fingerprint": "<redacted>","#,
188+
)];
189+
190+
#[test]
191+
fn output() {
192+
let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Gitlab);
193+
insta::with_settings!({filters => FINGERPRINT_FILTERS}, {
194+
insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
195+
});
196+
}
197+
198+
#[test]
199+
fn syntax_errors() {
200+
let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Gitlab);
201+
insta::with_settings!({filters => FINGERPRINT_FILTERS}, {
202+
insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
203+
});
204+
}
205+
}

0 commit comments

Comments
 (0)