Skip to content

Commit

Permalink
Add linting support to notebooks (Closes #1277) (#1313)
Browse files Browse the repository at this point in the history
This PR adds a few lines of code when collecting errors from a notebook
cell to also include lints.
  • Loading branch information
orpuente-MS authored Apr 23, 2024
1 parent 28817e4 commit 438d937
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 1 deletion.
33 changes: 32 additions & 1 deletion language_service/src/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ impl Compilation {
language_features,
);

// Compute new lints and append them to the errors Vec.
// Lints are only computed if the erros vector is empty. For performance
// reasons we don't want to waste time running lints every few keystrokes,
// if the user is in the middle of typing a statement, for example.
if errors.is_empty() {
let lints = qsc::linter::run_lints(&unit, Some(lints_config));
let lints: Vec<_> = lints
.into_iter()
.map(|lint| {
WithSource::from_map(&unit.sources, qsc::compile::ErrorKind::Lint(lint))
})
.collect();
errors.extend(lints);
}

let package_id = package_store.insert(unit);
let unit = package_store
.get(package_id)
Expand Down Expand Up @@ -106,6 +121,7 @@ impl Compilation {
cells: I,
target_profile: Profile,
language_features: LanguageFeatures,
lints_config: &[LintConfig],
) -> Self
where
I: Iterator<Item = (Arc<str>, Arc<str>)>,
Expand Down Expand Up @@ -138,6 +154,21 @@ impl Compilation {
.get(package_id)
.expect("expected to find user package");

// Compute new lints and append them to the errors Vec.
// Lints are only computed if the erros vector is empty. For performance
// reasons we don't want to waste time running lints every few keystrokes,
// if the user is in the middle of typing a statement, for example.
if errors.is_empty() {
let lints = qsc::linter::run_lints(unit, Some(lints_config));
let lints: Vec<_> = lints
.into_iter()
.map(|lint| {
WithSource::from_map(&unit.sources, qsc::compile::ErrorKind::Lint(lint))
})
.collect();
errors.extend(lints);
}

run_fir_passes(
&mut errors,
target_profile,
Expand Down Expand Up @@ -234,7 +265,7 @@ impl Compilation {
lints_config,
),
CompilationKind::Notebook => {
Self::new_notebook(sources, target_profile, language_features)
Self::new_notebook(sources, target_profile, language_features, lints_config)
}
};
self.package_store = new.package_store;
Expand Down
4 changes: 4 additions & 0 deletions language_service/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ impl<'a> CompilationStateUpdater<'a> {
}),
configuration.target_profile,
notebook_configuration.language_features.unwrap_or_default(),
notebook_configuration
.lints_config
.as_ref()
.unwrap_or(&vec![]),
);

state.compilations.insert(
Expand Down
62 changes: 62 additions & 0 deletions language_service/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,68 @@ fn notebook_document_errors() {
);
}

#[test]
fn notebook_document_lints() {
let errors = RefCell::new(Vec::new());
let mut updater = new_updater(&errors);

updater.update_notebook_document(
"notebook.ipynb",
&NotebookMetadata::default(),
[
("cell1", 1, "operation Foo() : Unit { let x = 4;;;; }"),
("cell2", 1, "operation Bar() : Unit { let y = 5 / 0; }"),
]
.into_iter(),
);

expect_errors(
&errors,
&expect![[r#"
[
(
"cell1",
Some(
1,
),
[
Lint(
Lint {
span: Span {
lo: 35,
hi: 38,
},
level: Warn,
message: "redundant semicolons",
help: "remove the redundant semicolons",
},
),
],
),
(
"cell2",
Some(
1,
),
[
Lint(
Lint {
span: Span {
lo: 74,
hi: 79,
},
level: Warn,
message: "attempt to divide by zero",
help: "division by zero is not allowed",
},
),
],
),
]
"#]],
);
}

#[test]
fn notebook_update_remove_cell_clears_errors() {
let errors = RefCell::new(Vec::new());
Expand Down

0 comments on commit 438d937

Please sign in to comment.