fix(lsp): lint-ignore directives follow leading comments

Author: nayeemrmn

cli/lsp/analysis.rs

@@ -1100,51 +1100,59 @@ impl CodeActionCollection {
       .actions
       .push(CodeActionKind::DenoLint(ignore_error_action));
 
-    // Disable a lint error for the entire file.
-    let maybe_ignore_comment = module
+    let parsed_source = module
       .open_data
       .as_ref()
-      .and_then(|d| d.parsed_source.as_ref())
-      .and_then(|ps| {
-        let ps = ps.as_ref().ok()?;
-        // Note: we can use ps.get_leading_comments() but it doesn't
-        // work when shebang is present at the top of the file.
-        ps.comments().get_vec().iter().find_map(|c| {
-          let comment_text = c.text.trim();
-          comment_text.split_whitespace().next().and_then(|prefix| {
-            if prefix == "deno-lint-ignore-file" {
-              Some(c.clone())
-            } else {
-              None
-            }
-          })
+      .and_then(|d| d.parsed_source.as_ref()?.as_ref().ok());
+    let next_leading_comment_range = {
+      let line = parsed_source
+        .and_then(|ps| {
+          let last_comment = ps.get_leading_comments()?.iter().last()?;
+          Some(module.text_info().line_index(last_comment.end()) as u32 + 1)
         })
-      });
-
-    let mut new_text = format!("// deno-lint-ignore-file {code}\n");
-    let mut range = lsp::Range {
-      start: lsp::Position {
-        line: 0,
-        character: 0,
-      },
-      end: lsp::Position {
-        line: 0,
-        character: 0,
-      },
+        .unwrap_or(0);
+      let position = lsp::Position { line, character: 0 };
+      lsp::Range {
+        start: position,
+        end: position,
+      }
     };
+
+    // Disable a lint error for the entire file.
+    let maybe_ignore_comment = parsed_source.and_then(|ps| {
+      // Note: we can use ps.get_leading_comments() but it doesn't
+      // work when shebang is present at the top of the file.
+      ps.comments().get_vec().iter().find_map(|c| {
+        let comment_text = c.text.trim();
+        comment_text.split_whitespace().next().and_then(|prefix| {
+          if prefix == "deno-lint-ignore-file" {
+            Some(c.clone())
+          } else {
+            None
+          }
+        })
+      })
+    });
+
+    let new_text;
+    let range;
     // If ignore file comment already exists, append the lint code
     // to the existing comment.
     if let Some(ignore_comment) = maybe_ignore_comment {
       new_text = format!(" {code}");
       // Get the end position of the comment.
-      let line = text_info.line_and_column_index(ignore_comment.end());
+      let index = text_info.line_and_column_index(ignore_comment.end());
       let position = lsp::Position {
-        line: line.line_index as u32,
-        character: line.column_index as u32,
+        line: index.line_index as u32,
+        character: index.column_index as u32,
+      };
+      range = lsp::Range {
+        start: position,
+        end: position,
       };
-      // Set the edit range to the end of the comment.
-      range.start = position;
-      range.end = position;
+    } else {
+      new_text = format!("// deno-lint-ignore-file {code}\n");
+      range = next_leading_comment_range;
     }
 
     let mut changes = HashMap::new();
@@ -1172,16 +1180,7 @@ impl CodeActionCollection {
       uri.clone(),
       vec![lsp::TextEdit {
         new_text: "// deno-lint-ignore-file\n".to_string(),
-        range: lsp::Range {
-          start: lsp::Position {
-            line: 0,
-            character: 0,
-          },
-          end: lsp::Position {
-            line: 0,
-            character: 0,
-          },
-        },
+        range: next_leading_comment_range,
       }],
     );
     let ignore_file_action = lsp::CodeAction {

tests/integration/lsp_tests.rs

@@ -14026,32 +14026,26 @@ console.log(snake_case);
 #[timeout(300_000)]
 fn lsp_code_actions_lint_fixes() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  let file = temp_dir
+    .source_file("file.ts", "// Copyright x-y. MIT licence.\nwindow;\n");
   let mut client = context.new_lsp_command().build();
   client.initialize_default();
-  let diagnostics = client.did_open(json!({
-    "textDocument": {
-      "uri": "file:///a/file.ts",
-      "languageId": "typescript",
-      "version": 1,
-      "text": "window;",
-    }
-  }));
+  let diagnostics = client.did_open_file(&file);
   let diagnostics = diagnostics.all();
   let diagnostic = &diagnostics[0];
   let res = client.write_request(
     "textDocument/codeAction",
     json!({
-      "textDocument": {
-        "uri": "file:///a/file.ts"
-      },
+      "textDocument": { "uri": file.uri() },
       "range": {
-        "start": { "line": 0, "character": 0 },
-        "end": { "line": 0, "character": 6 }
+        "start": { "line": 1, "character": 0 },
+        "end": { "line": 1, "character": 6 },
       },
       "context": {
         "diagnostics": [diagnostic],
-        "only": ["quickfix"]
-      }
+        "only": ["quickfix"],
+      },
     }),
   );
   assert_eq!(
@@ -14062,61 +14056,61 @@ fn lsp_code_actions_lint_fixes() {
       "diagnostics": [diagnostic],
       "edit": {
         "changes": {
-          "file:///a/file.ts": [{
+          file.uri().as_str(): [{
             "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 6 }
+              "start": { "line": 1, "character": 0 },
+              "end": { "line": 1, "character": 6 },
             },
-            "newText": "globalThis"
-          }]
-        }
-      }
+            "newText": "globalThis",
+          }],
+        },
+      },
     }, {
       "title": "Disable no-window for this line",
       "kind": "quickfix",
       "diagnostics": [diagnostic],
       "edit": {
         "changes": {
-          "file:///a/file.ts": [{
+          file.uri().as_str(): [{
             "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 }
+              "start": { "line": 1, "character": 0 },
+              "end": { "line": 1, "character": 0 },
             },
-            "newText": "// deno-lint-ignore no-window\n"
-          }]
-        }
-      }
+            "newText": "// deno-lint-ignore no-window\n",
+          }],
+        },
+      },
     }, {
       "title": "Disable no-window for the entire file",
       "kind": "quickfix",
       "diagnostics": [diagnostic],
       "edit": {
         "changes": {
-          "file:///a/file.ts": [{
+          file.uri().as_str(): [{
             "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 }
+              "start": { "line": 1, "character": 0 },
+              "end": { "line": 1, "character": 0 },
             },
-            "newText": "// deno-lint-ignore-file no-window\n"
-          }]
-        }
-      }
+            "newText": "// deno-lint-ignore-file no-window\n",
+          }],
+        },
+      },
     }, {
       "title": "Ignore lint errors for the entire file",
       "kind": "quickfix",
       "diagnostics": [diagnostic],
       "edit": {
         "changes": {
-          "file:///a/file.ts": [{
+          file.uri().as_str(): [{
             "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 }
+              "start": { "line": 1, "character": 0 },
+              "end": { "line": 1, "character": 0 },
             },
-            "newText": "// deno-lint-ignore-file\n"
-          }]
-        }
-      }
-    }])
+            "newText": "// deno-lint-ignore-file\n",
+          }],
+        },
+      },
+    }]),
   );
   client.shutdown();
 }