Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint groups to Q# #2103

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions compiler/qsc/src/interpret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,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()
Expand Down
4 changes: 3 additions & 1 deletion compiler/qsc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
4 changes: 3 additions & 1 deletion compiler/qsc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
37 changes: 37 additions & 0 deletions compiler/qsc_linter/src/lint_groups.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use serde::{Deserialize, Serialize};

use crate::{LintKind, LintLevel};

/// 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),
]
}
}
}
}
67 changes: 60 additions & 7 deletions compiler/qsc_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -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()));
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions compiler/qsc_linter/src/linter/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{
lints::ast::{AstLint, CombinedAstLints},
Lint, LintConfig, LintLevel,
Lint, LintLevel,
};
use qsc_ast::{
ast::{
Expand All @@ -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
}
Expand Down Expand Up @@ -342,4 +342,4 @@ macro_rules! declare_ast_lints {

pub(crate) use declare_ast_lints;

use super::{Compilation, LintKind};
use super::{Compilation, LintConfig, LintKind};
10 changes: 5 additions & 5 deletions compiler/qsc_linter/src/linter/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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
}
Expand Down Expand Up @@ -264,4 +264,4 @@ macro_rules! declare_hir_lints {

pub(crate) use declare_hir_lints;

use super::{Compilation, LintKind};
use super::{Compilation, LintConfig, LintKind};
50 changes: 44 additions & 6 deletions compiler/qsc_linter/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions compiler/qsc_project/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)]
Expand Down
4 changes: 2 additions & 2 deletions compiler/qsc_project/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>,
}
Expand Down
Loading
Loading