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" }
   ]
}
```

Author: orpuente-MS

compiler/qsc/src/interpret.rs

@@ -408,10 +408,12 @@ impl Interpreter {
                 compile_unit,
                 // see https://github.com/microsoft/qsharp/pull/1627 for context
                 // on why we override this config
-                Some(&[qsc_linter::LintConfig {
-                    kind: LintKind::Hir(HirLint::NeedlessOperation),
-                    level: LintLevel::Warn,
-                }]),
+                Some(&[qsc_linter::LintOrGroupConfig::Lint(
+                    qsc_linter::LintConfig {
+                        kind: LintKind::Hir(HirLint::NeedlessOperation),
+                        level: LintLevel::Warn,
+                    },
+                )]),
             )
         } else {
             Vec::new()

compiler/qsc/src/lib.rs

@@ -58,7 +58,9 @@ pub use qsc_eval::{
 };
 
 pub mod linter {
-    pub use qsc_linter::{run_lints, LintConfig, LintKind, LintLevel};
+    pub use qsc_linter::{
+        run_lints, GroupConfig, LintConfig, LintKind, LintLevel, LintOrGroupConfig,
+    };
 }
 
 pub use qsc_doc_gen::{display, generate_docs};

compiler/qsc_linter/src/lib.rs

@@ -61,10 +61,12 @@
 
 #![deny(missing_docs)]
 
+mod lint_groups;
 mod linter;
 mod lints;
 #[cfg(test)]
 mod tests;
 
-pub use linter::{run_lints, Lint, LintConfig, LintKind, LintLevel};
+pub use lint_groups::GroupConfig;
+pub use linter::{run_lints, Lint, LintConfig, LintKind, LintLevel, LintOrGroupConfig};
 pub use lints::{ast::AstLint, hir::HirLint};

compiler/qsc_linter/src/lint_groups.rs

@@ -0,0 +1,39 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+use crate::{LintKind, LintLevel};
+use serde::{Deserialize, Serialize};
+
+/// End-user configuration for a lint group.
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
+pub struct GroupConfig {
+    #[serde(rename = "group")]
+    /// The lint group.
+    pub lint_group: LintGroup,
+    /// The group level.
+    pub level: LintLevel,
+}
+
+#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq)]
+#[serde(rename_all = "camelCase")]
+pub enum LintGroup {
+    Deprecations,
+}
+
+impl LintGroup {
+    pub fn unfold(self) -> Vec<LintKind> {
+        use crate::AstLint::*;
+        use crate::HirLint::*;
+        match self {
+            LintGroup::Deprecations => {
+                vec![
+                    LintKind::Ast(DeprecatedNewtype),
+                    LintKind::Ast(DeprecatedSet),
+                    LintKind::Hir(DeprecatedFunctionConstructor),
+                    LintKind::Hir(DeprecatedWithOperator),
+                    LintKind::Hir(DeprecatedDoubleColonOperator),
+                ]
+            }
+        }
+    }
+}

compiler/qsc_linter/src/linter.rs

@@ -5,11 +5,15 @@ pub(crate) mod ast;
 pub(crate) mod hir;
 
 use self::{ast::run_ast_lints, hir::run_hir_lints};
-use crate::lints::{ast::AstLint, hir::HirLint};
+use crate::{
+    lints::{ast::AstLint, hir::HirLint},
+    GroupConfig,
+};
 use miette::{Diagnostic, LabeledSpan};
 use qsc_data_structures::span::Span;
 use qsc_frontend::compile::{CompileUnit, PackageStore};
 use qsc_hir::hir::{Item, ItemId};
+use rustc_hash::FxHashMap;
 use serde::{Deserialize, Serialize};
 use std::fmt::Display;
 
@@ -19,7 +23,7 @@ use std::fmt::Display;
 pub fn run_lints(
     package_store: &PackageStore,
     compile_unit: &CompileUnit,
-    config: Option<&[LintConfig]>,
+    config: Option<&[LintOrGroupConfig]>,
 ) -> Vec<Lint> {
     let mut lints = run_lints_without_deduplication(package_store, compile_unit, config);
     remove_duplicates(&mut lints);
@@ -33,22 +37,61 @@ pub fn run_lints(
 pub(crate) fn run_lints_without_deduplication(
     package_store: &PackageStore,
     compile_unit: &CompileUnit,
-    config: Option<&[LintConfig]>,
+    config: Option<&[LintOrGroupConfig]>,
 ) -> Vec<Lint> {
     let compilation = Compilation {
         package_store,
         compile_unit,
     };
 
-    let mut ast_lints = run_ast_lints(&compile_unit.ast.package, config, compilation);
-    let mut hir_lints = run_hir_lints(&compile_unit.package, config, compilation);
+    let unfolded_config = config.map(unfold_groups);
+
+    let mut ast_lints = run_ast_lints(
+        &compile_unit.ast.package,
+        unfolded_config.as_deref(),
+        compilation,
+    );
+    let mut hir_lints = run_hir_lints(
+        &compile_unit.package,
+        unfolded_config.as_deref(),
+        compilation,
+    );
 
     let mut lints = Vec::new();
     lints.append(&mut ast_lints);
     lints.append(&mut hir_lints);
     lints
 }
 
+/// Unfolds groups into lists of lints. Specific lints override group configs.
+pub(crate) fn unfold_groups(config: &[LintOrGroupConfig]) -> Vec<LintConfig> {
+    let mut config_map: FxHashMap<LintKind, LintLevel> = FxHashMap::default();
+
+    // Unfold groups in the order they appear.
+    for elt in config {
+        if let LintOrGroupConfig::Group(group) = elt {
+            for lint in group.lint_group.unfold() {
+                config_map.insert(lint, group.level);
+            }
+        }
+    }
+
+    // Specific lints override group configs.
+    for elt in config {
+        if let LintOrGroupConfig::Lint(lint) = elt {
+            config_map.insert(lint.kind, lint.level);
+        }
+    }
+
+    config_map
+        .iter()
+        .map(|(kind, level)| LintConfig {
+            kind: *kind,
+            level: *level,
+        })
+        .collect()
+}
+
 pub(crate) fn remove_duplicates<T: Eq + std::hash::Hash + Clone>(vec: &mut Vec<T>) {
     let mut seen = rustc_hash::FxHashSet::default();
     vec.retain(|x| seen.insert(x.clone()));
@@ -211,11 +254,21 @@ impl Display for LintLevel {
     }
 }
 
-/// End-user configuration for each lint level.
+/// End-user configuration for a specific lint or a lint group.
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
+#[serde(untagged)]
+pub enum LintOrGroupConfig {
+    /// An specific lint configuration.
+    Lint(LintConfig),
+    /// A lint group configuration.
+    Group(GroupConfig),
+}
+
+/// End-user configuration for a specific lint.
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
 pub struct LintConfig {
     #[serde(rename = "lint")]
-    /// Represents the lint name.
+    /// The lint name.
     pub kind: LintKind,
     /// The lint level.
     pub level: LintLevel,

compiler/qsc_linter/src/linter/ast.rs

@@ -3,7 +3,7 @@
 
 use crate::{
     lints::ast::{AstLint, CombinedAstLints},
-    Lint, LintConfig, LintLevel,
+    Lint, LintLevel,
 };
 use qsc_ast::{
     ast::{
@@ -24,9 +24,9 @@ pub fn run_ast_lints(
     let config: Vec<(AstLint, LintLevel)> = config
         .unwrap_or(&[])
         .iter()
-        .filter_map(|lint_config| {
-            if let LintKind::Ast(kind) = lint_config.kind {
-                Some((kind, lint_config.level))
+        .filter_map(|lint| {
+            if let LintKind::Ast(kind) = lint.kind {
+                Some((kind, lint.level))
             } else {
                 None
             }
@@ -342,4 +342,4 @@ macro_rules! declare_ast_lints {
 
 pub(crate) use declare_ast_lints;
 
-use super::{Compilation, LintKind};
+use super::{Compilation, LintConfig, LintKind};

compiler/qsc_linter/src/linter/hir.rs

@@ -3,7 +3,7 @@
 
 use crate::{
     lints::hir::{CombinedHirLints, HirLint},
-    Lint, LintConfig, LintLevel,
+    Lint, LintLevel,
 };
 use qsc_hir::{
     hir::{Block, CallableDecl, Expr, Ident, Item, Package, Pat, QubitInit, SpecDecl, Stmt},
@@ -21,9 +21,9 @@ pub fn run_hir_lints(
     let config: Vec<(HirLint, LintLevel)> = config
         .unwrap_or(&[])
         .iter()
-        .filter_map(|lint_config| {
-            if let LintKind::Hir(kind) = lint_config.kind {
-                Some((kind, lint_config.level))
+        .filter_map(|lint| {
+            if let LintKind::Hir(kind) = lint.kind {
+                Some((kind, lint.level))
             } else {
                 None
             }
@@ -264,4 +264,4 @@ macro_rules! declare_hir_lints {
 
 pub(crate) use declare_hir_lints;
 
-use super::{Compilation, LintKind};
+use super::{Compilation, LintConfig, LintKind};

compiler/qsc_linter/src/tests.rs

@@ -2,8 +2,9 @@
 // Licensed under the MIT License.
 
 use crate::{
+    lint_groups::LintGroup,
     linter::{remove_duplicates, run_lints_without_deduplication},
-    Lint, LintLevel,
+    Lint, LintLevel, LintOrGroupConfig,
 };
 use expect_test::{expect, Expect};
 use indoc::indoc;
@@ -101,6 +102,35 @@ fn set_keyword_lint() {
     );
 }
 
+#[test]
+fn lint_group() {
+    check_with_config(
+        &wrap_in_callable("newtype Foo = ()", CallableKind::Operation),
+        Some(&[LintOrGroupConfig::Group(crate::GroupConfig {
+            lint_group: LintGroup::Deprecations,
+            level: LintLevel::Error,
+        })]),
+        &expect![[r#"
+            [
+                SrcLint {
+                    source: "newtype Foo = ()",
+                    level: Error,
+                    message: "deprecated `newtype` declarations",
+                    help: "`newtype` declarations are deprecated, use `struct` instead",
+                    code_action_edits: [],
+                },
+                SrcLint {
+                    source: "RunProgram",
+                    level: Allow,
+                    message: "operation does not contain any quantum operations",
+                    help: "this callable can be declared as a function instead",
+                    code_action_edits: [],
+                },
+            ]
+        "#]],
+    );
+}
+
 #[test]
 fn multiple_lints() {
     check(
@@ -759,7 +789,7 @@ fn check_that_hir_lints_are_deduplicated_in_operations_with_multiple_specializat
     );
 }
 
-fn compile_and_collect_lints(source: &str) -> Vec<Lint> {
+fn compile_and_collect_lints(source: &str, config: Option<&[LintOrGroupConfig]>) -> Vec<Lint> {
     let mut store = PackageStore::new(compile::core());
     let std = store.insert(compile::std(&store, TargetCapabilityFlags::all()));
     let sources = SourceMap::new([("source.qs".into(), source.into())], None);
@@ -774,13 +804,21 @@ fn compile_and_collect_lints(source: &str) -> Vec<Lint> {
 
     let id = store.insert(unit);
     let unit = store.get(id).expect("user package should exist");
-
-    run_lints_without_deduplication(&store, unit, None)
+    run_lints_without_deduplication(&store, unit, config)
 }
 
 fn check(source: &str, expected: &Expect) {
     let source = wrap_in_namespace(source);
-    let actual: Vec<_> = compile_and_collect_lints(&source)
+    let actual: Vec<_> = compile_and_collect_lints(&source, None)
+        .into_iter()
+        .map(|lint| SrcLint::from(&lint, &source))
+        .collect();
+    expected.assert_debug_eq(&actual);
+}
+
+fn check_with_config(source: &str, config: Option<&[LintOrGroupConfig]>, expected: &Expect) {
+    let source = wrap_in_namespace(source);
+    let actual: Vec<_> = compile_and_collect_lints(&source, config)
         .into_iter()
         .map(|lint| SrcLint::from(&lint, &source))
         .collect();
@@ -789,7 +827,7 @@ fn check(source: &str, expected: &Expect) {
 
 fn check_with_deduplication(source: &str, expected: &Expect) {
     let source = wrap_in_namespace(source);
-    let mut lints = compile_and_collect_lints(&source);
+    let mut lints = compile_and_collect_lints(&source, None);
     remove_duplicates(&mut lints);
     let actual: Vec<_> = lints
         .into_iter()

compiler/qsc_project/src/manifest.rs

@@ -9,7 +9,7 @@ use std::{
     fs::{self, DirEntry, FileType},
 };
 
-pub use qsc_linter::LintConfig;
+pub use qsc_linter::LintOrGroupConfig;
 use rustc_hash::FxHashMap;
 use serde::{Deserialize, Serialize};
 use std::path::PathBuf;
@@ -25,7 +25,7 @@ pub struct Manifest {
     #[serde(default)]
     pub language_features: Vec<String>,
     #[serde(default)]
-    pub lints: Vec<LintConfig>,
+    pub lints: Vec<LintOrGroupConfig>,
     #[serde(default)]
     pub dependencies: FxHashMap<String, PackageRef>,
     #[serde(default)]

compiler/qsc_project/src/project.rs

@@ -9,7 +9,7 @@ use async_trait::async_trait;
 use futures::FutureExt;
 use miette::Diagnostic;
 use qsc_data_structures::language_features::LanguageFeatures;
-use qsc_linter::LintConfig;
+use qsc_linter::LintOrGroupConfig;
 use rustc_hash::FxHashMap;
 use std::{
     cell::RefCell,
@@ -33,7 +33,7 @@ pub struct Project {
     /// configuration settings.
     pub package_graph_sources: PackageGraphSources,
     /// Lint configuration for the project, typically comes from the root `qsharp.json`.
-    pub lints: Vec<LintConfig>,
+    pub lints: Vec<LintOrGroupConfig>,
     /// Any errors encountered while loading the project.
     pub errors: Vec<Error>,
 }