Merge pull request github#12434 from aibaars/diagnostics-links

Ruby: add some links to diagnostic messages

Author: aibaars

ruby/extractor/src/diagnostics.rs

@@ -214,6 +214,14 @@ fn longest_backtick_sequence_length(text: &str) -> usize {
     }
     result
 }
+/**
+ * An argument of a diagnostic message format string. A message argument is either a "code" snippet or a link.
+ */
+pub enum MessageArg<'a> {
+    Code(&'a str),
+    Link(&'a str, &'a str),
+}
+
 impl DiagnosticMessage {
     pub fn full_error_message(&self) -> String {
         match &self.location {
@@ -236,26 +244,39 @@ impl DiagnosticMessage {
         self
     }
 
-    pub fn message(&mut self, text: &str, args: &[&str]) -> &mut Self {
+    pub fn message(&mut self, text: &str, args: &[MessageArg]) -> &mut Self {
         let parts = text.split("{}");
-        let args = args.iter().chain(std::iter::repeat(&""));
         let mut plain = String::with_capacity(2 * text.len());
         let mut markdown = String::with_capacity(2 * text.len());
-        for (p, a) in parts.zip(args) {
+        for (i, p) in parts.enumerate() {
             plain.push_str(p);
-            plain.push_str(a);
             markdown.push_str(p);
-            if a.len() > 0 {
-                let count = longest_backtick_sequence_length(a) + 1;
-                markdown.push_str(&"`".repeat(count));
-                if count > 1 {
-                    markdown.push_str(" ");
+            match args.get(i) {
+                Some(MessageArg::Code(t)) => {
+                    plain.push_str(t);
+                    if t.len() > 0 {
+                        let count = longest_backtick_sequence_length(t) + 1;
+                        markdown.push_str(&"`".repeat(count));
+                        if count > 1 {
+                            markdown.push_str(" ");
+                        }
+                        markdown.push_str(t);
+                        if count > 1 {
+                            markdown.push_str(" ");
+                        }
+                        markdown.push_str(&"`".repeat(count));
+                    }
                 }
-                markdown.push_str(a);
-                if count > 1 {
-                    markdown.push_str(" ");
+                Some(MessageArg::Link(text, url)) => {
+                    plain.push_str(text);
+                    self.help_link(url);
+                    markdown.push_str("[");
+                    markdown.push_str(text);
+                    markdown.push_str("](");
+                    markdown.push_str(url);
+                    markdown.push_str(")");
                 }
-                markdown.push_str(&"`".repeat(count));
+                None => {}
             }
         }
         self.text(&plain);
@@ -323,14 +344,17 @@ fn test_message() {
     let mut m = DiagnosticLoggers::new("foo")
         .logger()
         .new_entry("id", "name");
-    m.message("hello: {}", &["hello"]);
+    m.message("hello: {}", &[MessageArg::Code("hello")]);
     assert_eq!("hello: hello", m.plaintext_message);
     assert_eq!("hello: `hello`", m.markdown_message);
 
     let mut m = DiagnosticLoggers::new("foo")
         .logger()
         .new_entry("id", "name");
-    m.message("hello with backticks: {}", &["oh `hello`!"]);
+    m.message(
+        "hello with backticks: {}",
+        &[MessageArg::Code("oh `hello`!")],
+    );
     assert_eq!("hello with backticks: oh `hello`!", m.plaintext_message);
     assert_eq!(
         "hello with backticks: `` oh `hello`! ``",

ruby/extractor/src/extractor.rs

@@ -277,7 +277,7 @@ impl<'a> Visitor<'a> {
     fn record_parse_error_for_node(
         &mut self,
         message: &str,
-        args: &[&str],
+        args: &[diagnostics::MessageArg],
         node: Node,
         status_page: bool,
     ) {
@@ -306,17 +306,17 @@ impl<'a> Visitor<'a> {
     fn enter_node(&mut self, node: Node) -> bool {
         if node.is_missing() {
             self.record_parse_error_for_node(
-                "A parse error occurred (expected {} symbol). Check the syntax of the file using the {} command. If the file is invalid, correct the error or exclude the file from analysis.",
-                &[node.kind(), "ruby -c"],
+                "A parse error occurred (expected {} symbol). Check the syntax of the file. If the file is invalid, correct the error or {} the file from analysis.",
+                &[diagnostics::MessageArg::Code(node.kind()), diagnostics::MessageArg::Link("exclude", "https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning")],
                 node,
                 true,
             );
             return false;
         }
         if node.is_error() {
             self.record_parse_error_for_node(
-                "A parse error occurred. Check the syntax of the file using the {} command. If the file is invalid, correct the error or exclude the file from analysis.",
-                &["ruby -c"],
+                "A parse error occurred. Check the syntax of the file. If the file is invalid, correct the error or {} the file from analysis.",
+                &[diagnostics::MessageArg::Link("exclude", "https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning")],
                 node,
                 true,
             );
@@ -407,7 +407,10 @@ impl<'a> Visitor<'a> {
                         .new_entry("parse-error", "Parse error")
                         .severity(diagnostics::Severity::Error)
                         .location(self.path, start_line, start_column, end_line, end_column)
-                        .message("Unknown table type: {}", &[node.kind()]),
+                        .message(
+                            "Unknown table type: {}",
+                            &[diagnostics::MessageArg::Code(node.kind())],
+                        ),
                 );
 
                 valid = false;
@@ -458,10 +461,10 @@ impl<'a> Visitor<'a> {
                     self.record_parse_error_for_node(
                         "Type mismatch for field {}::{} with type {} != {}",
                         &[
-                            node.kind(),
-                            child_node.field_name.unwrap_or("child"),
-                            &format!("{:?}", child_node.type_name),
-                            &format!("{:?}", field.type_info),
+                            diagnostics::MessageArg::Code(node.kind()),
+                            diagnostics::MessageArg::Code(child_node.field_name.unwrap_or("child")),
+                            diagnostics::MessageArg::Code(&format!("{:?}", child_node.type_name)),
+                            diagnostics::MessageArg::Code(&format!("{:?}", field.type_info)),
                         ],
                         *node,
                         false,
@@ -471,9 +474,9 @@ impl<'a> Visitor<'a> {
                 self.record_parse_error_for_node(
                     "Value for unknown field: {}::{} and type {}",
                     &[
-                        node.kind(),
-                        &child_node.field_name.unwrap_or("child"),
-                        &format!("{:?}", child_node.type_name),
+                        diagnostics::MessageArg::Code(node.kind()),
+                        diagnostics::MessageArg::Code(&child_node.field_name.unwrap_or("child")),
+                        diagnostics::MessageArg::Code(&format!("{:?}", child_node.type_name)),
                     ],
                     *node,
                     false,
@@ -512,7 +515,10 @@ impl<'a> Visitor<'a> {
                         if !*has_index && index > 0 {
                             self.record_parse_error_for_node(
                                 "Too many values for field: {}::{}",
-                                &[node.kind(), table_name],
+                                &[
+                                    diagnostics::MessageArg::Code(node.kind()),
+                                    diagnostics::MessageArg::Code(table_name),
+                                ],
                                 *node,
                                 false,
                             );
@@ -629,8 +635,11 @@ fn location_for(visitor: &mut Visitor, n: Node) -> (usize, usize, usize, usize)
                     .diagnostics_writer
                     .new_entry("internal-error", "Internal error")
                     .message(
-                        "Cannot correct end column value: end_byte index {} is not in range [1,{}]",
-                        &[&index.to_string(), &source.len().to_string()],
+                        "Cannot correct end column value: end_byte index {} is not in range [1,{}].",
+                        &[
+                            diagnostics::MessageArg::Code(&index.to_string()),
+                            diagnostics::MessageArg::Code(&source.len().to_string()),
+                        ],
                     )
                     .severity(diagnostics::Severity::Error),
             );

ruby/extractor/src/main.rs

@@ -74,7 +74,10 @@ fn main() -> std::io::Result<()> {
             main_thread_logger.write(
                 main_thread_logger
                     .new_entry("configuration-error", "Configuration error")
-                    .message("{}; defaulting to 1 thread.", &[&e])
+                    .message(
+                        "{}; defaulting to 1 thread.",
+                        &[diagnostics::MessageArg::Code(&e)],
+                    )
                     .severity(diagnostics::Severity::Warning),
             );
             1
@@ -95,7 +98,7 @@ fn main() -> std::io::Result<()> {
             main_thread_logger.write(
                 main_thread_logger
                     .new_entry("configuration-error", "Configuration error")
-                    .message("{}; using gzip.", &[&e])
+                    .message("{}; using gzip.", &[diagnostics::MessageArg::Code(&e)])
                     .severity(diagnostics::Severity::Warning),
             );
             trap::Compression::Gzip
@@ -203,11 +206,15 @@ fn main() -> std::io::Result<()> {
                                                 )
                                                 .file(&path.to_string_lossy())
                                                 .message(
-                                                    "Could not decode the file contents as {}: {}. The contents of the file must match the character encoding specified in the {} directive.",
-                                                    &[&encoding_name, &msg, "encoding:"],
+                                                    "Could not decode the file contents as {}: {}. The contents of the file must match the character encoding specified in the {} {}.",
+                                                    &[
+                                                        diagnostics::MessageArg::Code(&encoding_name),
+                                                        diagnostics::MessageArg::Code(&msg),
+                                                        diagnostics::MessageArg::Code("encoding:"),
+                                                        diagnostics::MessageArg::Link("directive", "https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
+                                                    ],
                                                 )
                                                 .status_page()
-                                                .help_link("https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
                                                 .severity(diagnostics::Severity::Warning),
                                         );
                                     }
@@ -219,11 +226,14 @@ fn main() -> std::io::Result<()> {
                                     .new_entry("unknown-character-encoding", "Unknown character encoding")
                                     .file(&path.to_string_lossy())
                                     .message(
-                                        "Unknown character encoding {} in {} directive.",
-                                        &[&encoding_name, "#encoding:"],
+                                        "Unknown character encoding {} in {} {}.",
+                                        &[
+                                            diagnostics::MessageArg::Code(&encoding_name),
+                                            diagnostics::MessageArg::Code("#encoding:"),
+                                            diagnostics::MessageArg::Link("directive", "https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
+                                        ],
                                     )
                                     .status_page()
-                                    .help_link("https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
                                     .severity(diagnostics::Severity::Warning),
                             );
                         }

ruby/ql/test/query-tests/diagnostics/ExtractionErrors.expected

@@ -1 +1 @@
-| src/not_ruby.rb:5:25:5:26 | A parse error occurred. Check the syntax of the file using the ruby -c command. If the file is invalid, correct the error or exclude the file from analysis. | Extraction failed in src/not_ruby.rb with error A parse error occurred. Check the syntax of the file using the ruby -c command. If the file is invalid, correct the error or exclude the file from analysis. | 2 |
+| src/not_ruby.rb:5:25:5:26 | A parse error occurred. Check the syntax of the file. If the file is invalid, correct the error or exclude the file from analysis. | Extraction failed in src/not_ruby.rb with error A parse error occurred. Check the syntax of the file. If the file is invalid, correct the error or exclude the file from analysis. | 2 |