Skip to content

Commit a529cc2

Browse files
authored
Add lint groups to Q# (#2103)
This PR adds the necessary infrastructure to define lint groups. It also adds the `deprecation` lint group, which allows the user to enable all deprecation lints at the same time, which are set to `allow` by default. A lint group can be added to the `qsharp.json` manifest as follows: ```json { "lints": [ { "group": "deprecations", "level": "warn" } ] } ```
1 parent 7e71ef5 commit a529cc2

File tree

17 files changed

+270
-92
lines changed

17 files changed

+270
-92
lines changed

compiler/qsc/src/interpret.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,12 @@ impl Interpreter {
408408
compile_unit,
409409
// see https://github.com/microsoft/qsharp/pull/1627 for context
410410
// on why we override this config
411-
Some(&[qsc_linter::LintConfig {
412-
kind: LintKind::Hir(HirLint::NeedlessOperation),
413-
level: LintLevel::Warn,
414-
}]),
411+
Some(&[qsc_linter::LintOrGroupConfig::Lint(
412+
qsc_linter::LintConfig {
413+
kind: LintKind::Hir(HirLint::NeedlessOperation),
414+
level: LintLevel::Warn,
415+
},
416+
)]),
415417
)
416418
} else {
417419
Vec::new()

compiler/qsc/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ pub use qsc_eval::{
5858
};
5959

6060
pub mod linter {
61-
pub use qsc_linter::{run_lints, LintConfig, LintKind, LintLevel};
61+
pub use qsc_linter::{
62+
run_lints, GroupConfig, LintConfig, LintKind, LintLevel, LintOrGroupConfig,
63+
};
6264
}
6365

6466
pub use qsc_doc_gen::{display, generate_docs};

compiler/qsc_linter/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@
6161
6262
#![deny(missing_docs)]
6363

64+
mod lint_groups;
6465
mod linter;
6566
mod lints;
6667
#[cfg(test)]
6768
mod tests;
6869

69-
pub use linter::{run_lints, Lint, LintConfig, LintKind, LintLevel};
70+
pub use lint_groups::GroupConfig;
71+
pub use linter::{run_lints, Lint, LintConfig, LintKind, LintLevel, LintOrGroupConfig};
7072
pub use lints::{ast::AstLint, hir::HirLint};
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
use crate::{LintKind, LintLevel};
5+
use serde::{Deserialize, Serialize};
6+
7+
/// End-user configuration for a lint group.
8+
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
9+
pub struct GroupConfig {
10+
#[serde(rename = "group")]
11+
/// The lint group.
12+
pub lint_group: LintGroup,
13+
/// The group level.
14+
pub level: LintLevel,
15+
}
16+
17+
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq)]
18+
#[serde(rename_all = "camelCase")]
19+
pub enum LintGroup {
20+
Deprecations,
21+
}
22+
23+
impl LintGroup {
24+
pub fn unfold(self) -> Vec<LintKind> {
25+
use crate::AstLint::*;
26+
use crate::HirLint::*;
27+
match self {
28+
LintGroup::Deprecations => {
29+
vec![
30+
LintKind::Ast(DeprecatedNewtype),
31+
LintKind::Ast(DeprecatedSet),
32+
LintKind::Hir(DeprecatedFunctionConstructor),
33+
LintKind::Hir(DeprecatedWithOperator),
34+
LintKind::Hir(DeprecatedDoubleColonOperator),
35+
]
36+
}
37+
}
38+
}
39+
}

compiler/qsc_linter/src/linter.rs

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@ pub(crate) mod ast;
55
pub(crate) mod hir;
66

77
use self::{ast::run_ast_lints, hir::run_hir_lints};
8-
use crate::lints::{ast::AstLint, hir::HirLint};
8+
use crate::{
9+
lints::{ast::AstLint, hir::HirLint},
10+
GroupConfig,
11+
};
912
use miette::{Diagnostic, LabeledSpan};
1013
use qsc_data_structures::span::Span;
1114
use qsc_frontend::compile::{CompileUnit, PackageStore};
1215
use qsc_hir::hir::{Item, ItemId};
16+
use rustc_hash::FxHashMap;
1317
use serde::{Deserialize, Serialize};
1418
use std::fmt::Display;
1519

@@ -19,7 +23,7 @@ use std::fmt::Display;
1923
pub fn run_lints(
2024
package_store: &PackageStore,
2125
compile_unit: &CompileUnit,
22-
config: Option<&[LintConfig]>,
26+
config: Option<&[LintOrGroupConfig]>,
2327
) -> Vec<Lint> {
2428
let mut lints = run_lints_without_deduplication(package_store, compile_unit, config);
2529
remove_duplicates(&mut lints);
@@ -33,22 +37,61 @@ pub fn run_lints(
3337
pub(crate) fn run_lints_without_deduplication(
3438
package_store: &PackageStore,
3539
compile_unit: &CompileUnit,
36-
config: Option<&[LintConfig]>,
40+
config: Option<&[LintOrGroupConfig]>,
3741
) -> Vec<Lint> {
3842
let compilation = Compilation {
3943
package_store,
4044
compile_unit,
4145
};
4246

43-
let mut ast_lints = run_ast_lints(&compile_unit.ast.package, config, compilation);
44-
let mut hir_lints = run_hir_lints(&compile_unit.package, config, compilation);
47+
let unfolded_config = config.map(unfold_groups);
48+
49+
let mut ast_lints = run_ast_lints(
50+
&compile_unit.ast.package,
51+
unfolded_config.as_deref(),
52+
compilation,
53+
);
54+
let mut hir_lints = run_hir_lints(
55+
&compile_unit.package,
56+
unfolded_config.as_deref(),
57+
compilation,
58+
);
4559

4660
let mut lints = Vec::new();
4761
lints.append(&mut ast_lints);
4862
lints.append(&mut hir_lints);
4963
lints
5064
}
5165

66+
/// Unfolds groups into lists of lints. Specific lints override group configs.
67+
pub(crate) fn unfold_groups(config: &[LintOrGroupConfig]) -> Vec<LintConfig> {
68+
let mut config_map: FxHashMap<LintKind, LintLevel> = FxHashMap::default();
69+
70+
// Unfold groups in the order they appear.
71+
for elt in config {
72+
if let LintOrGroupConfig::Group(group) = elt {
73+
for lint in group.lint_group.unfold() {
74+
config_map.insert(lint, group.level);
75+
}
76+
}
77+
}
78+
79+
// Specific lints override group configs.
80+
for elt in config {
81+
if let LintOrGroupConfig::Lint(lint) = elt {
82+
config_map.insert(lint.kind, lint.level);
83+
}
84+
}
85+
86+
config_map
87+
.iter()
88+
.map(|(kind, level)| LintConfig {
89+
kind: *kind,
90+
level: *level,
91+
})
92+
.collect()
93+
}
94+
5295
pub(crate) fn remove_duplicates<T: Eq + std::hash::Hash + Clone>(vec: &mut Vec<T>) {
5396
let mut seen = rustc_hash::FxHashSet::default();
5497
vec.retain(|x| seen.insert(x.clone()));
@@ -211,11 +254,21 @@ impl Display for LintLevel {
211254
}
212255
}
213256

214-
/// End-user configuration for each lint level.
257+
/// End-user configuration for a specific lint or a lint group.
258+
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
259+
#[serde(untagged)]
260+
pub enum LintOrGroupConfig {
261+
/// An specific lint configuration.
262+
Lint(LintConfig),
263+
/// A lint group configuration.
264+
Group(GroupConfig),
265+
}
266+
267+
/// End-user configuration for a specific lint.
215268
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
216269
pub struct LintConfig {
217270
#[serde(rename = "lint")]
218-
/// Represents the lint name.
271+
/// The lint name.
219272
pub kind: LintKind,
220273
/// The lint level.
221274
pub level: LintLevel,

compiler/qsc_linter/src/linter/ast.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use crate::{
55
lints::ast::{AstLint, CombinedAstLints},
6-
Lint, LintConfig, LintLevel,
6+
Lint, LintLevel,
77
};
88
use qsc_ast::{
99
ast::{
@@ -24,9 +24,9 @@ pub fn run_ast_lints(
2424
let config: Vec<(AstLint, LintLevel)> = config
2525
.unwrap_or(&[])
2626
.iter()
27-
.filter_map(|lint_config| {
28-
if let LintKind::Ast(kind) = lint_config.kind {
29-
Some((kind, lint_config.level))
27+
.filter_map(|lint| {
28+
if let LintKind::Ast(kind) = lint.kind {
29+
Some((kind, lint.level))
3030
} else {
3131
None
3232
}
@@ -342,4 +342,4 @@ macro_rules! declare_ast_lints {
342342

343343
pub(crate) use declare_ast_lints;
344344

345-
use super::{Compilation, LintKind};
345+
use super::{Compilation, LintConfig, LintKind};

compiler/qsc_linter/src/linter/hir.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use crate::{
55
lints::hir::{CombinedHirLints, HirLint},
6-
Lint, LintConfig, LintLevel,
6+
Lint, LintLevel,
77
};
88
use qsc_hir::{
99
hir::{Block, CallableDecl, Expr, Ident, Item, Package, Pat, QubitInit, SpecDecl, Stmt},
@@ -21,9 +21,9 @@ pub fn run_hir_lints(
2121
let config: Vec<(HirLint, LintLevel)> = config
2222
.unwrap_or(&[])
2323
.iter()
24-
.filter_map(|lint_config| {
25-
if let LintKind::Hir(kind) = lint_config.kind {
26-
Some((kind, lint_config.level))
24+
.filter_map(|lint| {
25+
if let LintKind::Hir(kind) = lint.kind {
26+
Some((kind, lint.level))
2727
} else {
2828
None
2929
}
@@ -264,4 +264,4 @@ macro_rules! declare_hir_lints {
264264

265265
pub(crate) use declare_hir_lints;
266266

267-
use super::{Compilation, LintKind};
267+
use super::{Compilation, LintConfig, LintKind};

compiler/qsc_linter/src/tests.rs

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// Licensed under the MIT License.
33

44
use crate::{
5+
lint_groups::LintGroup,
56
linter::{remove_duplicates, run_lints_without_deduplication},
6-
Lint, LintLevel,
7+
Lint, LintLevel, LintOrGroupConfig,
78
};
89
use expect_test::{expect, Expect};
910
use indoc::indoc;
@@ -101,6 +102,35 @@ fn set_keyword_lint() {
101102
);
102103
}
103104

105+
#[test]
106+
fn lint_group() {
107+
check_with_config(
108+
&wrap_in_callable("newtype Foo = ()", CallableKind::Operation),
109+
Some(&[LintOrGroupConfig::Group(crate::GroupConfig {
110+
lint_group: LintGroup::Deprecations,
111+
level: LintLevel::Error,
112+
})]),
113+
&expect![[r#"
114+
[
115+
SrcLint {
116+
source: "newtype Foo = ()",
117+
level: Error,
118+
message: "deprecated `newtype` declarations",
119+
help: "`newtype` declarations are deprecated, use `struct` instead",
120+
code_action_edits: [],
121+
},
122+
SrcLint {
123+
source: "RunProgram",
124+
level: Allow,
125+
message: "operation does not contain any quantum operations",
126+
help: "this callable can be declared as a function instead",
127+
code_action_edits: [],
128+
},
129+
]
130+
"#]],
131+
);
132+
}
133+
104134
#[test]
105135
fn multiple_lints() {
106136
check(
@@ -759,7 +789,7 @@ fn check_that_hir_lints_are_deduplicated_in_operations_with_multiple_specializat
759789
);
760790
}
761791

762-
fn compile_and_collect_lints(source: &str) -> Vec<Lint> {
792+
fn compile_and_collect_lints(source: &str, config: Option<&[LintOrGroupConfig]>) -> Vec<Lint> {
763793
let mut store = PackageStore::new(compile::core());
764794
let std = store.insert(compile::std(&store, TargetCapabilityFlags::all()));
765795
let sources = SourceMap::new([("source.qs".into(), source.into())], None);
@@ -774,13 +804,21 @@ fn compile_and_collect_lints(source: &str) -> Vec<Lint> {
774804

775805
let id = store.insert(unit);
776806
let unit = store.get(id).expect("user package should exist");
777-
778-
run_lints_without_deduplication(&store, unit, None)
807+
run_lints_without_deduplication(&store, unit, config)
779808
}
780809

781810
fn check(source: &str, expected: &Expect) {
782811
let source = wrap_in_namespace(source);
783-
let actual: Vec<_> = compile_and_collect_lints(&source)
812+
let actual: Vec<_> = compile_and_collect_lints(&source, None)
813+
.into_iter()
814+
.map(|lint| SrcLint::from(&lint, &source))
815+
.collect();
816+
expected.assert_debug_eq(&actual);
817+
}
818+
819+
fn check_with_config(source: &str, config: Option<&[LintOrGroupConfig]>, expected: &Expect) {
820+
let source = wrap_in_namespace(source);
821+
let actual: Vec<_> = compile_and_collect_lints(&source, config)
784822
.into_iter()
785823
.map(|lint| SrcLint::from(&lint, &source))
786824
.collect();
@@ -789,7 +827,7 @@ fn check(source: &str, expected: &Expect) {
789827

790828
fn check_with_deduplication(source: &str, expected: &Expect) {
791829
let source = wrap_in_namespace(source);
792-
let mut lints = compile_and_collect_lints(&source);
830+
let mut lints = compile_and_collect_lints(&source, None);
793831
remove_duplicates(&mut lints);
794832
let actual: Vec<_> = lints
795833
.into_iter()

compiler/qsc_project/src/manifest.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{
99
fs::{self, DirEntry, FileType},
1010
};
1111

12-
pub use qsc_linter::LintConfig;
12+
pub use qsc_linter::LintOrGroupConfig;
1313
use rustc_hash::FxHashMap;
1414
use serde::{Deserialize, Serialize};
1515
use std::path::PathBuf;
@@ -25,7 +25,7 @@ pub struct Manifest {
2525
#[serde(default)]
2626
pub language_features: Vec<String>,
2727
#[serde(default)]
28-
pub lints: Vec<LintConfig>,
28+
pub lints: Vec<LintOrGroupConfig>,
2929
#[serde(default)]
3030
pub dependencies: FxHashMap<String, PackageRef>,
3131
#[serde(default)]

compiler/qsc_project/src/project.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use async_trait::async_trait;
99
use futures::FutureExt;
1010
use miette::Diagnostic;
1111
use qsc_data_structures::language_features::LanguageFeatures;
12-
use qsc_linter::LintConfig;
12+
use qsc_linter::LintOrGroupConfig;
1313
use rustc_hash::FxHashMap;
1414
use std::{
1515
cell::RefCell,
@@ -33,7 +33,7 @@ pub struct Project {
3333
/// configuration settings.
3434
pub package_graph_sources: PackageGraphSources,
3535
/// Lint configuration for the project, typically comes from the root `qsharp.json`.
36-
pub lints: Vec<LintConfig>,
36+
pub lints: Vec<LintOrGroupConfig>,
3737
/// Any errors encountered while loading the project.
3838
pub errors: Vec<Error>,
3939
}

0 commit comments

Comments
 (0)