chore: multi line deno_lint diagnostics (#7303)

Cargo.lock

@@ -465,9 +465,9 @@ dependencies = [
 
 [[package]]
 name = "deno_lint"
-version = "0.1.26"
+version = "0.1.27"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "723604b9f2a203366c0f7e6cfc66e43f15439fa8cfd7e98582a0836c8af9ab56"
+checksum = "9818f45029f09d92a06a5dd372130c21cb054bc5f582f3f660089ac2c82e68b3"
 dependencies = [
  "lazy_static",
  "log 0.4.11",

cli/Cargo.toml

@@ -30,7 +30,7 @@ winapi = "0.3.9"
 [dependencies]
 deno_core = { path = "../core", version = "0.55.0" }
 deno_doc = { version = "0.1.3" }
-deno_lint = { version = "0.1.26", features = ["json"] }
+deno_lint = { version = "0.1.27", features = ["json"] }
 
 atty = "0.2.14"
 base64 = "0.12.3"

cli/lint.rs

@@ -73,10 +73,11 @@ pub async fn lint_files(
       let mut reporter = reporter_lock.lock().unwrap();
 
       match r {
-        Ok(file_diagnostics) => {
+        Ok((mut file_diagnostics, source)) => {
+          sort_diagnostics(&mut file_diagnostics);
           for d in file_diagnostics.iter() {
             has_error.store(true, Ordering::Relaxed);
-            reporter.visit(&d);
+            reporter.visit(&d, source.split('\n').collect());
           }
         }
         Err(err) => {
@@ -124,7 +125,9 @@ fn create_linter(syntax: Syntax, rules: Vec<Box<dyn LintRule>>) -> Linter {
     .build()
 }
 
-fn lint_file(file_path: PathBuf) -> Result<Vec<LintDiagnostic>, ErrBox> {
+fn lint_file(
+  file_path: PathBuf,
+) -> Result<(Vec<LintDiagnostic>, String), ErrBox> {
   let file_name = file_path.to_string_lossy().to_string();
   let source_code = fs::read_to_string(&file_path)?;
   let media_type = map_file_extension(&file_path);
@@ -133,9 +136,9 @@ fn lint_file(file_path: PathBuf) -> Result<Vec<LintDiagnostic>, ErrBox> {
   let lint_rules = rules::get_recommended_rules();
   let mut linter = create_linter(syntax, lint_rules);
 
-  let file_diagnostics = linter.lint(file_name, source_code)?;
+  let file_diagnostics = linter.lint(file_name, source_code.clone())?;
 
-  Ok(file_diagnostics)
+  Ok((file_diagnostics, source_code))
 }
 
 /// Lint stdin and write result to stdout.
@@ -159,13 +162,13 @@ fn lint_stdin(json: bool) -> Result<(), ErrBox> {
   let mut has_error = false;
   let pseudo_file_name = "_stdin.ts";
   match linter
-    .lint(pseudo_file_name.to_string(), source)
+    .lint(pseudo_file_name.to_string(), source.clone())
     .map_err(|e| e.into())
   {
     Ok(diagnostics) => {
       for d in diagnostics {
         has_error = true;
-        reporter.visit(&d);
+        reporter.visit(&d, source.split('\n').collect());
       }
     }
     Err(err) => {
@@ -184,7 +187,7 @@ fn lint_stdin(json: bool) -> Result<(), ErrBox> {
 }
 
 trait LintReporter {
-  fn visit(&mut self, d: &LintDiagnostic);
+  fn visit(&mut self, d: &LintDiagnostic, source_lines: Vec<&str>);
   fn visit_error(&mut self, file_path: &str, err: &ErrBox);
   fn close(&mut self);
 }
@@ -206,24 +209,21 @@ impl PrettyLintReporter {
 }
 
 impl LintReporter for PrettyLintReporter {
-  fn visit(&mut self, d: &LintDiagnostic) {
+  fn visit(&mut self, d: &LintDiagnostic, source_lines: Vec<&str>) {
     self.lint_count += 1;
 
     let pretty_message =
       format!("({}) {}", colors::gray(&d.code), d.message.clone());
 
-    let message = fmt_errors::format_stack(
-      true,
+    let message = format_diagnostic(
       &pretty_message,
-      Some(&d.line_src),
-      Some(d.location.col as i64),
-      Some((d.location.col + d.snippet_length) as i64),
-      &[fmt_errors::format_location(
+      &source_lines,
+      d.range.clone(),
+      &fmt_errors::format_location(
         &d.filename,
-        d.location.line as i64,
-        d.location.col as i64,
-      )],
-      0,
+        d.range.start.line as i64,
+        d.range.start.col as i64,
+      ),
     );
 
     eprintln!("{}\n", message);
@@ -243,6 +243,46 @@ impl LintReporter for PrettyLintReporter {
   }
 }
 
+pub fn format_diagnostic(
+  message_line: &str,
+  source_lines: &[&str],
+  range: deno_lint::diagnostic::Range,
+  formatted_location: &str,
+) -> String {
+  let mut lines = vec![];
+
+  for i in range.start.line..=range.end.line {
+    lines.push(source_lines[i - 1].to_string());
+    if range.start.line == range.end.line {
+      lines.push(format!(
+        "{}{}",
+        " ".repeat(range.start.col),
+        colors::red(&"^".repeat(range.end.col - range.start.col))
+      ));
+    } else {
+      let line_len = source_lines[i - 1].len();
+      if range.start.line == i {
+        lines.push(format!(
+          "{}{}",
+          " ".repeat(range.start.col),
+          colors::red(&"^".repeat(line_len - range.start.col))
+        ));
+      } else if range.end.line == i {
+        lines.push(format!("{}", colors::red(&"^".repeat(range.end.col))));
+      } else if line_len != 0 {
+        lines.push(format!("{}", colors::red(&"^".repeat(line_len))));
+      }
+    }
+  }
+
+  format!(
+    "{}\n{}\n    at {}",
+    message_line,
+    lines.join("\n"),
+    formatted_location
+  )
+}
+
 #[derive(Serialize)]
 struct JsonLintReporter {
   diagnostics: Vec<LintDiagnostic>,
@@ -259,7 +299,7 @@ impl JsonLintReporter {
 }
 
 impl LintReporter for JsonLintReporter {
-  fn visit(&mut self, d: &LintDiagnostic) {
+  fn visit(&mut self, d: &LintDiagnostic, _source_lines: Vec<&str>) {
     self.diagnostics.push(d.clone());
   }
 
@@ -271,16 +311,26 @@ impl LintReporter for JsonLintReporter {
   }
 
   fn close(&mut self) {
-    // Sort so that we guarantee a deterministic output which is useful for tests
-    self.diagnostics.sort_by_key(|key| get_sort_key(&key));
-
+    sort_diagnostics(&mut self.diagnostics);
     let json = serde_json::to_string_pretty(&self);
     eprintln!("{}", json.unwrap());
   }
 }
 
-pub fn get_sort_key(a: &LintDiagnostic) -> String {
-  let location = &a.location;
-
-  return format!("{}:{}:{}", a.filename, location.line, location.col);
+fn sort_diagnostics(diagnostics: &mut Vec<LintDiagnostic>) {
+  // Sort so that we guarantee a deterministic output which is useful for tests
+  diagnostics.sort_by(|a, b| {
+    use std::cmp::Ordering;
+    let file_order = a.filename.cmp(&b.filename);
+    match file_order {
+      Ordering::Equal => {
+        let line_order = a.range.start.line.cmp(&b.range.start.line);
+        match line_order {
+          Ordering::Equal => a.range.start.col.cmp(&b.range.start.col),
+          _ => line_order,
+        }
+      }
+      _ => file_order,
+    }
+  });
 }

cli/tests/lint/expected_from_stdin_json.out

@@ -1,15 +1,19 @@
 {
   "diagnostics": [
     {
-      "location": {
-        "line": 1,
-        "col": 7
+      "range": {
+        "start": {
+          "line": 1,
+          "col": 7
+        },
+        "end": {
+          "line": 1,
+          "col": 10
+        }
       },
       "filename": "_stdin.ts",
       "message": "`any` type is not allowed",
-      "code": "no-explicit-any",
-      "line_src": "let a: any;",
-      "snippet_length": 3
+      "code": "no-explicit-any"
     }
   ],
   "errors": []

cli/tests/lint/expected_json.out

@@ -1,37 +1,49 @@
 {
   "diagnostics": [
     {
-      "location": {
-        "line": 1,
-        "col": 0
+      "range": {
+        "start": {
+          "line": 1,
+          "col": 0
+        },
+        "end": {
+          "line": 1,
+          "col": 19
+        }
       },
-      "filename": "[WILDCARD]",
+      "filename": "[WILDCARD]file1.js",
       "message": "Ignore directive requires lint rule code",
-      "code": "ban-untagged-ignore",
-      "line_src": "// deno-lint-ignore",
-      "snippet_length": 19
+      "code": "ban-untagged-ignore"
     },
     {
-      "location": {
-        "line": 2,
-        "col": 14
+      "range": {
+        "start": {
+          "line": 2,
+          "col": 14
+        },
+        "end": {
+          "line": 2,
+          "col": 16
+        }
       },
-      "filename": "[WILDCARD]",
+      "filename": "[WILDCARD]file1.js",
       "message": "Empty block statement",
-      "code": "no-empty",
-      "line_src": "while (false) {}",
-      "snippet_length": 2
+      "code": "no-empty"
     },
     {
-      "location": {
-        "line": 3,
-        "col": 12
+      "range": {
+        "start": {
+          "line": 3,
+          "col": 12
+        },
+        "end": {
+          "line": 3,
+          "col": 14
+        }
       },
-      "filename": "[WILDCARD]",
+      "filename": "[WILDCARD]file2.ts",
       "message": "Empty block statement",
-      "code": "no-empty",
-      "line_src": "} catch (e) {}",
-      "snippet_length": 2
+      "code": "no-empty"
     }
   ],
   "errors": [