Render sub-diagnostics in the GitHub output format

crates/ruff/tests/cli/snapshots/cli__lint__output_format_github.snap

@@ -16,7 +16,7 @@ info:
 success: false
 exit_code: 1
 ----- stdout -----
-::error title=ruff (F401),file=[TMP]/input.py,line=1,col=8,endLine=1,endColumn=10::input.py:1:8: F401 `os` imported but unused
+::error title=ruff (F401),file=[TMP]/input.py,line=1,col=8,endLine=1,endColumn=10::input.py:1:8: F401 `os` imported but unused%0A  help: Remove unused import: `os`
 ::error title=ruff (F821),file=[TMP]/input.py,line=2,col=5,endLine=2,endColumn=6::input.py:2:5: F821 Undefined name `y`
 ::error title=ruff (invalid-syntax),file=[TMP]/input.py,line=3,col=1,endLine=3,endColumn=6::input.py:3:1: invalid-syntax: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)
 

crates/ruff_db/src/diagnostic/mod.rs

@@ -317,17 +317,7 @@ impl Diagnostic {
 
     /// Returns all annotations, skipping the first primary annotation.
     pub fn secondary_annotations(&self) -> impl Iterator<Item = &Annotation> {
-        let mut seen_primary = false;
-        self.inner.annotations.iter().filter(move |ann| {
-            if seen_primary {
-                true
-            } else if ann.is_primary {
-                seen_primary = true;
-                false
-            } else {
-                true
-            }
-        })
+        secondary_annotations(self.inner.annotations.iter())
     }
 
     pub fn sub_diagnostics(&self) -> &[SubDiagnostic] {
@@ -669,6 +659,11 @@ impl SubDiagnostic {
         &self.inner.annotations
     }
 
+    /// Returns all annotations, skipping the first primary annotation.
+    pub fn secondary_annotations(&self) -> impl Iterator<Item = &Annotation> {
+        secondary_annotations(self.inner.annotations.iter())
+    }
+
     /// Returns a mutable borrow of the annotations of this sub-diagnostic.
     pub fn annotations_mut(&mut self) -> impl Iterator<Item = &mut Annotation> {
         self.inner.annotations.iter_mut()
@@ -707,6 +702,10 @@ impl SubDiagnostic {
             ConciseMessage::Both { main, annotation }
         }
     }
+
+    pub(crate) fn severity(&self) -> SubDiagnosticSeverity {
+        self.inner.severity
+    }
 }
 
 #[derive(Debug, Clone, Eq, PartialEq, Hash, get_size2::GetSize)]
@@ -716,6 +715,23 @@ struct SubDiagnosticInner {
     annotations: Vec<Annotation>,
 }
 
+/// Returns all annotations, skipping the first primary annotation.
+fn secondary_annotations<'a>(
+    annotations: impl Iterator<Item = &'a Annotation>,
+) -> impl Iterator<Item = &'a Annotation> {
+    let mut seen_primary = false;
+    annotations.filter(move |ann| {
+        if seen_primary {
+            true
+        } else if ann.is_primary {
+            seen_primary = true;
+            false
+        } else {
+            true
+        }
+    })
+}
+
 /// A pointer to a subsequence in the end user's input.
 ///
 /// Also known as an annotation, the pointer can optionally contain a short

crates/ruff_db/src/diagnostic/render.rs

@@ -2798,6 +2798,16 @@ watermelon
             self
         }
 
+        /// Adds a sub-diagnostic constructed with this diagnostic's environment.
+        fn sub(
+            mut self,
+            f: impl Fn(&mut TestEnvironment) -> SubDiagnostic,
+        ) -> DiagnosticBuilder<'e> {
+            let sub = f(self.env);
+            self.diag.sub(sub);
+            self
+        }
+
         /// Set the documentation URL for the diagnostic.
         pub(super) fn documentation_url(mut self, url: impl Into<String>) -> DiagnosticBuilder<'e> {
             self.diag.set_documentation_url(Some(url.into()));
@@ -2901,7 +2911,7 @@ def fibonacci(n):
     elif n == 1:
         return 1
     else:
-        return fibonacci(n - 1) + fibonacci(n - 2)
+        return fibonaccii(n - 1) + fibonacci(n - 2)
 "#,
         );
         env.add("undef.py", r"if a == 1: pass");
@@ -2940,6 +2950,26 @@ def fibonacci(n):
                 .noqa_offset(TextSize::from(3))
                 .documentation_url("https://docs.astral.sh/ruff/rules/undefined-name")
                 .build(),
+            env.builder(
+                "undefined-name",
+                Severity::Error,
+                "Undefined name `fibonaccii`",
+            )
+            .primary("fib.py", "12:15", "12:25", "")
+            .secondary_code("F821")
+            .noqa_offset(ruff_text_size::TextSize::from(0))
+            .documentation_url("https://docs.astral.sh/ruff/rules/undefined-name")
+            .secondary("fib.py", "12:35", "12:36", "")
+            .sub(|env| {
+                env.sub_builder(
+                    SubDiagnosticSeverity::Info,
+                    "Did you mean to import it from `/some/path/def.py`?",
+                )
+                .primary("fib.py", "4:4", "4:13", "`fibonacci` is defined here")
+                .secondary("fib.py", "5:4", "5", "`fibonacci` is documented here")
+                .build()
+            })
+            .build(),
         ];
 
         (env, diagnostics)
@@ -2973,36 +3003,6 @@ if call(foo
         (env, diagnostics)
     }
 
-    /// Create Ruff-style diagnostics with sub-diagnostics for testing the various output formats.
-    pub(crate) fn create_sub_diagnostics(
-        format: DiagnosticFormat,
-    ) -> (TestEnvironment, Vec<Diagnostic>) {
-        let mut env = TestEnvironment::new();
-        env.add("/some/path/def.py", "def f(): pass");
-        env.add("call.py", "f()");
-        env.format(format);
-
-        let mut primary_diagnostic = env
-            .builder("undefined-name", Severity::Error, "Undefined name `f`")
-            .primary("call.py", "1:0", "1:1", "")
-            .secondary_code("F821")
-            .noqa_offset(ruff_text_size::TextSize::from(0))
-            .documentation_url("https://docs.astral.sh/ruff/rules/undefined-name")
-            .build();
-
-        let sub_diagnostic = env
-            .sub_builder(
-                SubDiagnosticSeverity::Info,
-                "Did you mean to import it from `/some/path/def.py`?",
-            )
-            .primary("/some/path/def.py", "1:4", "1:5", "`f` is defined here")
-            .build();
-
-        primary_diagnostic.sub(sub_diagnostic);
-
-        (env, vec![primary_diagnostic])
-    }
-
     /// A Jupyter notebook for testing diagnostics.
     ///
     ///

crates/ruff_db/src/diagnostic/render/concise.rs

@@ -141,6 +141,7 @@ mod tests {
         fib.py:1:8: error[unused-import] `os` imported but unused
         fib.py:6:5: error[unused-variable] Local variable `x` is assigned to but never used
         undef.py:1:4: error[undefined-name] Undefined name `a`
+        fib.py:12:16: error[undefined-name] Undefined name `fibonaccii`
         ");
     }
 
@@ -154,6 +155,7 @@ mod tests {
         fib.py:1:8: F401 [*] `os` imported but unused
         fib.py:6:5: F841 [*] Local variable `x` is assigned to but never used
         undef.py:1:4: F821 Undefined name `a`
+        fib.py:12:16: F821 Undefined name `fibonaccii`
         ");
     }
 
@@ -168,6 +170,7 @@ mod tests {
         fib.py:1:8: F401 [*] `os` imported but unused
         fib.py:6:5: F841 [*] Local variable `x` is assigned to but never used
         undef.py:1:4: F821 Undefined name `a`
+        fib.py:12:16: F821 Undefined name `fibonaccii`
         ");
     }
 

crates/ruff_db/src/diagnostic/render/full.rs

@@ -338,6 +338,25 @@ mod tests {
         1 | if a == 1: pass
           |    ^
           |
+
+        error[undefined-name]: Undefined name `fibonaccii`
+          --> fib.py:12:16
+           |
+        10 |         return 1
+        11 |     else:
+        12 |         return fibonaccii(n - 1) + fibonacci(n - 2)
+           |                ^^^^^^^^^^          -
+           |
+        info: Did you mean to import it from `/some/path/def.py`?
+         --> fib.py:4:5
+          |
+        4 | def fibonacci(n):
+          |     ^^^^^^^^^ `fibonacci` is defined here
+        5 |     """Compute the nth number in the Fibonacci sequence."""
+          |     ------------------------------------------------------- `fibonacci` is documented here
+        6 |     x = 1
+        7 |     if n == 0:
+          |
         "#);
     }
 
@@ -401,6 +420,25 @@ mod tests {
         1 | if a == 1: pass
           |    ^
           |
+
+        F821 Undefined name `fibonaccii`
+          --> fib.py:12:16
+           |
+        10 |         return 1
+        11 |     else:
+        12 |         return fibonaccii(n - 1) + fibonacci(n - 2)
+           |                ^^^^^^^^^^          -
+           |
+        info: Did you mean to import it from `/some/path/def.py`?
+         --> fib.py:4:5
+          |
+        4 | def fibonacci(n):
+          |     ^^^^^^^^^ `fibonacci` is defined here
+        5 |     """Compute the nth number in the Fibonacci sequence."""
+          |     ------------------------------------------------------- `fibonacci` is documented here
+        6 |     x = 1
+        7 |     if n == 0:
+          |
         "#);
     }
 

crates/ruff_db/src/diagnostic/render/github.rs

@@ -1,4 +1,8 @@
-use crate::diagnostic::{Diagnostic, FileResolver, Severity};
+use ruff_text_size::TextRange;
+
+use crate::diagnostic::{
+    Annotation, Diagnostic, FileResolver, Severity, SubDiagnosticSeverity, UnifiedFile,
+};
 
 pub(super) struct GithubRenderer<'a> {
     resolver: &'a dyn FileResolver,
@@ -87,13 +91,97 @@ impl<'a> GithubRenderer<'a> {
                 write!(f, "{id}:", id = diagnostic.id())?;
             }
 
-            writeln!(f, " {}", diagnostic.concise_message())?;
+            write!(f, " {}", diagnostic.concise_message())?;
+
+            // After rendering the main diagnostic, render its secondary annotations and
+            // sub-diagnostics. Note that lines within a single diagnostic must be separated by
+            // URL-encoded newlines (`%0A`) to render properly in GitHub annotations.
+            for annotation in diagnostic.secondary_annotations().filter_map(|annotation| {
+                GithubAnnotation::from_annotation(annotation, self.resolver)
+            }) {
+                write!(f, "%0A{annotation}")?;
+            }
+
+            for subdiagnostic in diagnostic.sub_diagnostics() {
+                let severity = match subdiagnostic.severity() {
+                    SubDiagnosticSeverity::Help => "help",
+                    SubDiagnosticSeverity::Info => "info",
+                    SubDiagnosticSeverity::Warning => "warning",
+                    SubDiagnosticSeverity::Error | SubDiagnosticSeverity::Fatal => "error",
+                };
+                if let Some(annotation) = subdiagnostic.primary_annotation()
+                    && let span = annotation.get_span()
+                    && let file = span.file()
+                    && let Some(range) = span.range()
+                {
+                    let diagnostic_source = file.diagnostic_source(self.resolver);
+                    let source_code = diagnostic_source.as_source_code();
+                    let message = subdiagnostic.concise_message();
+                    let start_location = source_code.line_column(range.start());
+                    write!(
+                        f,
+                        "%0A  {path}:{row}:{column}: {severity}: {message}",
+                        path = file.relative_path(self.resolver).display(),
+                        row = start_location.line,
+                        column = start_location.column,
+                    )?;
+                } else {
+                    write!(f, "%0A  {severity}: {}", subdiagnostic.concise_message())?;
+                }
+
+                for annotation in subdiagnostic
+                    .secondary_annotations()
+                    .filter_map(|annotation| {
+                        GithubAnnotation::from_annotation(annotation, self.resolver)
+                    })
+                {
+                    write!(f, "%0A  {annotation}")?;
+                }
+            }
+
+            writeln!(f)?;
         }
 
         Ok(())
     }
 }
 
+struct GithubAnnotation<'a> {
+    message: &'a str,
+    range: TextRange,
+    file: &'a UnifiedFile,
+    resolver: &'a dyn FileResolver,
+}
+
+impl<'a> GithubAnnotation<'a> {
+    fn from_annotation(annotation: &'a Annotation, resolver: &'a dyn FileResolver) -> Option<Self> {
+        let span = annotation.get_span();
+        Some(Self {
+            message: annotation.get_message()?,
+            range: span.range()?,
+            file: span.file(),
+            resolver,
+        })
+    }
+}
+
+impl std::fmt::Display for GithubAnnotation<'_> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let diagnostic_source = self.file.diagnostic_source(self.resolver);
+        let source_code = diagnostic_source.as_source_code();
+        let start_location = source_code.line_column(self.range.start());
+        write!(
+            f,
+            "  {path}:{row}:{column}:",
+            path = self.file.relative_path(self.resolver).display(),
+            row = start_location.line,
+            column = start_location.column,
+        )?;
+
+        write!(f, " {message}", message = self.message)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::diagnostic::{

crates/ruff_db/src/diagnostic/render/junit.rs

@@ -184,9 +184,7 @@ impl std::io::Write for FmtAdapter<'_> {
 mod tests {
     use crate::diagnostic::{
         DiagnosticFormat,
-        render::tests::{
-            create_diagnostics, create_sub_diagnostics, create_syntax_error_diagnostics,
-        },
+        render::tests::{create_diagnostics, create_syntax_error_diagnostics},
     };
 
     #[test]
@@ -195,12 +193,6 @@ mod tests {
         insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
     }
 
-    #[test]
-    fn sub_diagnostics() {
-        let (env, diagnostics) = create_sub_diagnostics(DiagnosticFormat::Junit);
-        insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
-    }
-
     #[test]
     fn syntax_errors() {
         let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Junit);

crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__azure__tests__output.snap

@@ -5,3 +5,4 @@ expression: env.render_diagnostics(&diagnostics)
 ##vso[task.logissue type=error;sourcepath=/fib.py;linenumber=1;columnnumber=8;code=F401;]`os` imported but unused
 ##vso[task.logissue type=error;sourcepath=/fib.py;linenumber=6;columnnumber=5;code=F841;]Local variable `x` is assigned to but never used
 ##vso[task.logissue type=error;sourcepath=/undef.py;linenumber=1;columnnumber=4;code=F821;]Undefined name `a`
+##vso[task.logissue type=error;sourcepath=/fib.py;linenumber=12;columnnumber=16;code=F821;]Undefined name `fibonaccii`

crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__github__tests__output.snap

@@ -2,6 +2,7 @@
 source: crates/ruff_db/src/diagnostic/render/github.rs
 expression: env.render_diagnostics(&diagnostics)
 ---
-::error title=ty (F401),file=/fib.py,line=1,col=8,endLine=1,endColumn=10::fib.py:1:8: F401 `os` imported but unused
-::error title=ty (F841),file=/fib.py,line=6,col=5,endLine=6,endColumn=6::fib.py:6:5: F841 Local variable `x` is assigned to but never used
+::error title=ty (F401),file=/fib.py,line=1,col=8,endLine=1,endColumn=10::fib.py:1:8: F401 `os` imported but unused%0A  help: Remove unused import: `os`
+::error title=ty (F841),file=/fib.py,line=6,col=5,endLine=6,endColumn=6::fib.py:6:5: F841 Local variable `x` is assigned to but never used%0A  help: Remove assignment to unused variable `x`
 ::error title=ty (F821),file=/undef.py,line=1,col=4,endLine=1,endColumn=5::undef.py:1:4: F821 Undefined name `a`
+::error title=ty (F821),file=/fib.py,line=12,col=16,endLine=12,endColumn=26::fib.py:12:16: F821 Undefined name `fibonaccii`%0A  fib.py:4:5: info: Did you mean to import it from `/some/path/def.py`?: `fibonacci` is defined here%0A    fib.py:5:5: `fibonacci` is documented here