From 33dfa812ce7317f315a3ad826a31ff8b220b5970 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Wed, 11 Dec 2024 15:11:37 -0800 Subject: [PATCH 01/12] made `set` optional --- compiler/qsc_parse/src/expr.rs | 59 ++++++- compiler/qsc_parse/src/expr/tests.rs | 254 ++++++++++++++++++++++++--- 2 files changed, 282 insertions(+), 31 deletions(-) diff --git a/compiler/qsc_parse/src/expr.rs b/compiler/qsc_parse/src/expr.rs index 433dcf2d55..5647d7bbcc 100644 --- a/compiler/qsc_parse/src/expr.rs +++ b/compiler/qsc_parse/src/expr.rs @@ -43,6 +43,9 @@ struct MixfixOp { enum OpKind { Postfix(UnOp), Binary(BinOp, Assoc), + Assign, + AssignUpdate, + AssignBinary(BinOp), Ternary(TernOp, TokenKind, Assoc), Rich(fn(&mut ParserContext, Box) -> Result>), } @@ -131,6 +134,20 @@ fn expr_op(s: &mut ParserContext, context: OpContext) -> Result> { s.advance(); let kind = match op.kind { OpKind::Postfix(kind) => Box::new(ExprKind::UnOp(kind, lhs)), + OpKind::Assign => { + let rhs = expr_op(s, OpContext::Precedence(op.precedence))?; + Box::new(ExprKind::Assign(lhs, rhs)) + } + OpKind::AssignUpdate => { + let mid = expr(s)?; + token(s, TokenKind::LArrow)?; + let rhs = expr_op(s, OpContext::Precedence(op.precedence))?; + Box::new(ExprKind::AssignUpdate(lhs, mid, rhs)) + } + OpKind::AssignBinary(kind) => { + let rhs = expr_op(s, OpContext::Precedence(op.precedence))?; + Box::new(ExprKind::AssignOp(kind, lhs, rhs)) + } OpKind::Binary(kind, assoc) => { let precedence = next_precedence(op.precedence, assoc); let rhs = expr_op(s, OpContext::Precedence(precedence))?; @@ -212,7 +229,17 @@ fn expr_base(s: &mut ParserContext) -> Result> { } else if token(s, TokenKind::Keyword(Keyword::Return)).is_ok() { Ok(Box::new(ExprKind::Return(expr(s)?))) } else if token(s, TokenKind::Keyword(Keyword::Set)).is_ok() { - expr_set(s) + // Need to rewrite the span of the expr to include the `set` keyword. + return expr(s).map(|assign| { + Box::new(Expr { + id: assign.id, + span: Span { + lo, + hi: assign.span.hi, + }, + kind: assign.kind, + }) + }); } else if token(s, TokenKind::Keyword(Keyword::While)).is_ok() { Ok(Box::new(ExprKind::While(expr(s)?, stmt::parse_block(s)?))) } else if token(s, TokenKind::Keyword(Keyword::Within)).is_ok() { @@ -357,8 +384,8 @@ fn expr_array_core(s: &mut ParserContext) -> Result> { s.expect(WordKinds::Size); let second = expr(s)?; - if is_ident("size", &second.kind) && token(s, TokenKind::Eq).is_ok() { - let size = expr(s)?; + if let Some(size) = is_array_size(&second.kind) { + let size = Box::new(size.clone()); return Ok(Box::new(ExprKind::ArrayRepeat(first, size))); } @@ -369,8 +396,18 @@ fn expr_array_core(s: &mut ParserContext) -> Result> { Ok(Box::new(ExprKind::Array(items.into_boxed_slice()))) } -fn is_ident(name: &str, kind: &ExprKind) -> bool { - matches!(kind, ExprKind::Path(PathKind::Ok(path)) if path.segments.is_none() && path.name.name.as_ref() == name) +fn is_array_size(kind: &ExprKind) -> Option<&Expr> { + match kind { + ExprKind::Assign(lhs, rhs) => match lhs.kind.as_ref() { + ExprKind::Path(PathKind::Ok(path)) + if path.segments.is_none() && path.name.name.as_ref() == "size" => + { + Some(rhs) + } + _ => None, + }, + _ => None, + } } fn expr_range_prefix(s: &mut ParserContext) -> Result> { @@ -572,6 +609,18 @@ fn prefix_op(name: OpName) -> Option { #[allow(clippy::too_many_lines)] fn mixfix_op(name: OpName) -> Option { match name { + OpName::Token(TokenKind::Eq) => Some(MixfixOp { + kind: OpKind::Assign, + precedence: 0, + }), + OpName::Token(TokenKind::WSlashEq) => Some(MixfixOp { + kind: OpKind::AssignUpdate, + precedence: 0, + }), + OpName::Token(TokenKind::BinOpEq(kind)) => Some(MixfixOp { + kind: OpKind::AssignBinary(closed_bin_op(kind)), + precedence: 0, + }), OpName::Token(TokenKind::RArrow) => Some(MixfixOp { kind: OpKind::Rich(|s, input| lambda_op(s, *input, CallableKind::Function)), precedence: LAMBDA_PRECEDENCE, diff --git a/compiler/qsc_parse/src/expr/tests.rs b/compiler/qsc_parse/src/expr/tests.rs index 18069d761b..2389c5d305 100644 --- a/compiler/qsc_parse/src/expr/tests.rs +++ b/compiler/qsc_parse/src/expr/tests.rs @@ -5,6 +5,20 @@ use super::expr; use crate::tests::check; use expect_test::expect; +#[test] +fn foo() { + check( + expr, + "x = j = 3", + &expect![[r#" + Expr _id_ [0-9]: Assign: + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [4-9]: Assign: + Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "j") + Expr _id_ [8-9]: Lit: Int(3)"#]], + ); +} + #[test] fn lit_int() { check(expr, "123", &expect!["Expr _id_ [0-3]: Lit: Int(123)"]); @@ -699,6 +713,14 @@ fn set() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [8-9]: Path: Path _id_ [8-9] (Ident _id_ [8-9] "y")"#]], ); + check( + expr, + "x = y", + &expect![[r#" + Expr _id_ [0-5]: Assign: + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "y")"#]], + ); } #[test] @@ -711,6 +733,14 @@ fn set_hole() { Expr _id_ [4-5]: Hole Expr _id_ [8-9]: Lit: Int(1)"#]], ); + check( + expr, + "_ = 1", + &expect![[r#" + Expr _id_ [0-5]: Assign: + Expr _id_ [0-1]: Hole + Expr _id_ [4-5]: Lit: Int(1)"#]], + ); } #[test] @@ -727,6 +757,18 @@ fn set_hole_tuple() { Expr _id_ [14-15]: Lit: Int(1) Expr _id_ [17-18]: Lit: Int(2)"#]], ); + check( + expr, + "(x, _) = (1, 2)", + &expect![[r#" + Expr _id_ [0-15]: Assign: + Expr _id_ [0-6]: Tuple: + Expr _id_ [1-2]: Path: Path _id_ [1-2] (Ident _id_ [1-2] "x") + Expr _id_ [4-5]: Hole + Expr _id_ [9-15]: Tuple: + Expr _id_ [10-11]: Lit: Int(1) + Expr _id_ [13-14]: Lit: Int(2)"#]], + ); } #[test] @@ -747,6 +789,22 @@ fn set_hole_tuple_nested() { Expr _id_ [23-24]: Lit: Int(2) Expr _id_ [26-27]: Lit: Int(3)"#]], ); + check( + expr, + "(_, (x, _)) = (1, (2, 3))", + &expect![[r#" + Expr _id_ [0-25]: Assign: + Expr _id_ [0-11]: Tuple: + Expr _id_ [1-2]: Hole + Expr _id_ [4-10]: Tuple: + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "x") + Expr _id_ [8-9]: Hole + Expr _id_ [14-25]: Tuple: + Expr _id_ [15-16]: Lit: Int(1) + Expr _id_ [18-24]: Tuple: + Expr _id_ [19-20]: Lit: Int(2) + Expr _id_ [22-23]: Lit: Int(3)"#]], + ); } #[test] @@ -759,6 +817,14 @@ fn set_bitwise_and() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], ); + check( + expr, + "x &&&= y", + &expect![[r#" + Expr _id_ [0-8]: AssignOp (AndB): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y")"#]], + ); } #[test] @@ -771,6 +837,14 @@ fn set_logical_and() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], ); + check( + expr, + "x and= y", + &expect![[r#" + Expr _id_ [0-8]: AssignOp (AndL): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y")"#]], + ); } #[test] @@ -783,6 +857,14 @@ fn set_bitwise_or() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], ); + check( + expr, + "x |||= y", + &expect![[r#" + Expr _id_ [0-8]: AssignOp (OrB): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y")"#]], + ); } #[test] @@ -795,6 +877,14 @@ fn set_exp() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y")"#]], ); + check( + expr, + "x ^= y", + &expect![[r#" + Expr _id_ [0-6]: AssignOp (Exp): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y")"#]], + ); } #[test] @@ -807,6 +897,14 @@ fn set_bitwise_xor() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], ); + check( + expr, + "x ^^^= y", + &expect![[r#" + Expr _id_ [0-8]: AssignOp (XorB): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y")"#]], + ); } #[test] @@ -819,6 +917,14 @@ fn set_shr() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], ); + check( + expr, + "x >>>= y", + &expect![[r#" + Expr _id_ [0-8]: AssignOp (Shr): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y")"#]], + ); } #[test] @@ -831,6 +937,14 @@ fn set_shl() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], ); + check( + expr, + "x <<<= y", + &expect![[r#" + Expr _id_ [0-8]: AssignOp (Shl): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y")"#]], + ); } #[test] @@ -843,6 +957,14 @@ fn set_sub() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y")"#]], ); + check( + expr, + "x -= y", + &expect![[r#" + Expr _id_ [0-6]: AssignOp (Sub): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y")"#]], + ); } #[test] @@ -855,6 +977,14 @@ fn set_logical_or() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [10-11]: Path: Path _id_ [10-11] (Ident _id_ [10-11] "y")"#]], ); + check( + expr, + "x or= y", + &expect![[r#" + Expr _id_ [0-7]: AssignOp (OrL): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [6-7]: Path: Path _id_ [6-7] (Ident _id_ [6-7] "y")"#]], + ); } #[test] @@ -867,6 +997,14 @@ fn set_mod() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y")"#]], ); + check( + expr, + "x %= y", + &expect![[r#" + Expr _id_ [0-6]: AssignOp (Mod): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y")"#]], + ); } #[test] @@ -879,6 +1017,14 @@ fn set_add() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y")"#]], ); + check( + expr, + "x += y", + &expect![[r#" + Expr _id_ [0-6]: AssignOp (Add): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y")"#]], + ); } #[test] @@ -891,6 +1037,14 @@ fn set_div() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y")"#]], ); + check( + expr, + "x /= y", + &expect![[r#" + Expr _id_ [0-6]: AssignOp (Div): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y")"#]], + ); } #[test] @@ -903,6 +1057,14 @@ fn set_mul() { Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y")"#]], ); + check( + expr, + "x *= y", + &expect![[r#" + Expr _id_ [0-6]: AssignOp (Mul): + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y")"#]], + ); } #[test] @@ -916,6 +1078,15 @@ fn set_with_update() { Expr _id_ [10-11]: Path: Path _id_ [10-11] (Ident _id_ [10-11] "i") Expr _id_ [15-16]: Path: Path _id_ [15-16] (Ident _id_ [15-16] "y")"#]], ); + check( + expr, + "x w/= i <- y", + &expect![[r#" + Expr _id_ [0-12]: AssignUpdate: + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [6-7]: Path: Path _id_ [6-7] (Ident _id_ [6-7] "i") + Expr _id_ [11-12]: Path: Path _id_ [11-12] (Ident _id_ [11-12] "y")"#]], + ); } #[test] @@ -1073,19 +1244,10 @@ fn array_repeat_no_items() { expr, "[size = 3]", &expect![[r#" - Error( - Token( - Close( - Bracket, - ), - Eq, - Span { - lo: 6, - hi: 7, - }, - ), - ) - "#]], + Expr _id_ [0-10]: Array: + Expr _id_ [1-9]: Assign: + Expr _id_ [1-5]: Path: Path _id_ [1-5] (Ident _id_ [1-5] "size") + Expr _id_ [8-9]: Lit: Int(3)"#]], ); } @@ -1095,19 +1257,12 @@ fn array_repeat_two_items() { expr, "[1, 2, size = 3]", &expect![[r#" - Error( - Token( - Close( - Bracket, - ), - Eq, - Span { - lo: 12, - hi: 13, - }, - ), - ) - "#]], + Expr _id_ [0-16]: Array: + Expr _id_ [1-2]: Lit: Int(1) + Expr _id_ [4-5]: Lit: Int(2) + Expr _id_ [7-15]: Assign: + Expr _id_ [7-11]: Path: Path _id_ [7-11] (Ident _id_ [7-11] "size") + Expr _id_ [14-15]: Lit: Int(3)"#]], ); } @@ -2496,6 +2651,30 @@ fn duplicate_commas_in_pattern() { ), ]"#]], ); + check( + expr, + "(x,, y) = (1, 2)", + &expect![[r#" + Expr _id_ [0-16]: Assign: + Expr _id_ [0-7]: Tuple: + Expr _id_ [1-2]: Path: Path _id_ [1-2] (Ident _id_ [1-2] "x") + Expr _id_ [3-3]: Err + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y") + Expr _id_ [10-16]: Tuple: + Expr _id_ [11-12]: Lit: Int(1) + Expr _id_ [14-15]: Lit: Int(2) + + [ + Error( + MissingSeqEntry( + Span { + lo: 3, + hi: 3, + }, + ), + ), + ]"#]], + ); } #[test] @@ -2523,6 +2702,29 @@ fn invalid_initial_commas_in_pattern() { ), ]"#]], ); + check( + expr, + "(, x) = (1, 2)", + &expect![[r#" + Expr _id_ [0-14]: Assign: + Expr _id_ [0-5]: Tuple: + Expr _id_ [1-1]: Err + Expr _id_ [3-4]: Path: Path _id_ [3-4] (Ident _id_ [3-4] "x") + Expr _id_ [8-14]: Tuple: + Expr _id_ [9-10]: Lit: Int(1) + Expr _id_ [12-13]: Lit: Int(2) + + [ + Error( + MissingSeqEntry( + Span { + lo: 1, + hi: 1, + }, + ), + ), + ]"#]], + ); } #[test] From a4742030833a106e768b8ef88e3ac57091e0aa86 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Wed, 11 Dec 2024 16:26:06 -0800 Subject: [PATCH 02/12] lint and code action --- compiler/qsc_linter/src/linter.rs | 2 +- compiler/qsc_linter/src/linter/ast.rs | 161 ++++++++++++++++---------- compiler/qsc_linter/src/lints/ast.rs | 38 +++++- compiler/qsc_linter/src/tests.rs | 28 ++++- compiler/qsc_parse/src/expr.rs | 23 ---- 5 files changed, 162 insertions(+), 90 deletions(-) diff --git a/compiler/qsc_linter/src/linter.rs b/compiler/qsc_linter/src/linter.rs index f6728a7689..1be9eefc38 100644 --- a/compiler/qsc_linter/src/linter.rs +++ b/compiler/qsc_linter/src/linter.rs @@ -26,7 +26,7 @@ pub fn run_lints( compile_unit, }; - let mut ast_lints = run_ast_lints(&compile_unit.ast.package, config); + 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 mut lints = Vec::new(); diff --git a/compiler/qsc_linter/src/linter/ast.rs b/compiler/qsc_linter/src/linter/ast.rs index 697998ad4c..e6610909e2 100644 --- a/compiler/qsc_linter/src/linter/ast.rs +++ b/compiler/qsc_linter/src/linter/ast.rs @@ -16,7 +16,11 @@ use qsc_ast::{ /// The entry point to the AST linter. It takes a [`qsc_ast::ast::Package`] /// as input and outputs a [`Vec`](Lint). #[must_use] -pub fn run_ast_lints(package: &qsc_ast::ast::Package, config: Option<&[LintConfig]>) -> Vec { +pub fn run_ast_lints( + package: &qsc_ast::ast::Package, + config: Option<&[LintConfig]>, + compilation: Compilation, +) -> Vec { let config: Vec<(AstLint, LintLevel)> = config .unwrap_or(&[]) .iter() @@ -29,7 +33,7 @@ pub fn run_ast_lints(package: &qsc_ast::ast::Package, config: Option<&[LintConfi }) .collect(); - let mut lints = CombinedAstLints::from_config(config); + let mut lints = CombinedAstLints::from_config(config, compilation); for node in &package.nodes { match node { @@ -46,23 +50,65 @@ pub fn run_ast_lints(package: &qsc_ast::ast::Package, config: Option<&[LintConfi /// The trait provides default empty implementations for the rest of the methods, /// which will be optimized to a no-op by the rust compiler. pub(crate) trait AstLintPass { - fn check_attr(&self, _attr: &Attr, _buffer: &mut Vec) {} - fn check_block(&self, _block: &Block, _buffer: &mut Vec) {} - fn check_callable_decl(&self, _callable_decl: &CallableDecl, _buffer: &mut Vec) {} - fn check_expr(&self, _expr: &Expr, _buffer: &mut Vec) {} - fn check_functor_expr(&self, _functor_expr: &FunctorExpr, _buffer: &mut Vec) {} - fn check_ident(&self, _ident: &Ident, _buffer: &mut Vec) {} - fn check_item(&self, _item: &Item, _buffer: &mut Vec) {} - fn check_namespace(&self, _namespace: &Namespace, _buffer: &mut Vec) {} - fn check_package(&self, _package: &Package, _buffer: &mut Vec) {} - fn check_pat(&self, _pat: &Pat, _buffer: &mut Vec) {} - fn check_path(&self, _path: &Path, _buffer: &mut Vec) {} - fn check_path_kind(&self, _path: &PathKind, _buffer: &mut Vec) {} - fn check_qubit_init(&self, _qubit_init: &QubitInit, _buffer: &mut Vec) {} - fn check_spec_decl(&self, _spec_decl: &SpecDecl, _buffer: &mut Vec) {} - fn check_stmt(&self, _stmt: &Stmt, _buffer: &mut Vec) {} - fn check_ty(&self, _ty: &Ty, _buffer: &mut Vec) {} - fn check_ty_def(&self, _ty_def: &TyDef, _buffer: &mut Vec) {} + fn check_attr(&self, _attr: &Attr, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_block(&self, _block: &Block, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_callable_decl( + &self, + _callable_decl: &CallableDecl, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_expr(&self, _expr: &Expr, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_functor_expr( + &self, + _functor_expr: &FunctorExpr, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_ident(&self, _ident: &Ident, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_item(&self, _item: &Item, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_namespace( + &self, + _namespace: &Namespace, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_package( + &self, + _package: &Package, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_pat(&self, _pat: &Pat, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_path(&self, _path: &Path, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_path_kind( + &self, + _path: &PathKind, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_qubit_init( + &self, + _qubit_init: &QubitInit, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_spec_decl( + &self, + _spec_decl: &SpecDecl, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_stmt(&self, _stmt: &Stmt, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_ty(&self, _ty: &Ty, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_ty_def(&self, _ty_def: &TyDef, _buffer: &mut Vec, _compilation: Compilation) {} } /// This macro allow us to declare lints while avoiding boilerplate. It does three things: @@ -82,7 +128,7 @@ macro_rules! declare_ast_lints { // This is a silly wrapper module to avoid contaminating the environment // calling the macro with unwanted imports. mod _ast_macro_expansion { - use crate::{linter::ast::{declare_ast_lints, AstLintPass}, Lint, LintLevel}; + use crate::{linter::Compilation, linter::ast::{declare_ast_lints, AstLintPass}, Lint, LintLevel}; use qsc_ast::{ ast::{ Attr, Block, CallableDecl, Expr, FunctorExpr, Ident, Item, Namespace, Package, Pat, Path, PathKind, @@ -106,25 +152,20 @@ macro_rules! declare_ast_lints { // Declare & implement a struct representing a lint. (@LINT_STRUCT $lint_name:ident, $default_level:expr, $msg:expr, $help:expr) => { + + #[derive(Default)] pub(crate) struct $lint_name { level: LintLevel, } - impl Default for $lint_name { - fn default() -> Self { - Self { level: Self::DEFAULT_LEVEL } - } - } - - impl From for $lint_name { - fn from(value: LintLevel) -> Self { - Self { level: value } - } - } - impl $lint_name { const DEFAULT_LEVEL: LintLevel = $default_level; + fn new() -> Self { + #[allow(clippy::needless_update)] + Self { level: Self::DEFAULT_LEVEL, ..Default::default() } + } + const fn lint_kind(&self) -> LintKind { LintKind::Ast(AstLint::$lint_name) } @@ -164,24 +205,26 @@ macro_rules! declare_ast_lints { /// Combined AST lints for speed. This combined lint allow us to /// evaluate all the lints in a single AST pass, instead of doing /// an individual pass for each lint in the linter. - pub(crate) struct CombinedAstLints { + pub(crate) struct CombinedAstLints<'compilation> { pub buffer: Vec, + pub compilation: Compilation<'compilation>, $($lint_name: $lint_name),* } - impl Default for CombinedAstLints { - fn default() -> Self { + impl<'compilation> CombinedAstLints<'compilation> { + fn new(compilation: Compilation<'compilation>) -> Self { Self { - buffer: Vec::default(), - $($lint_name: <$lint_name>::default()),* + buffer: Vec::new(), + compilation, + $($lint_name: <$lint_name>::new()),* } } } // Most of the calls here are empty methods and they get optimized at compile time to a no-op. - impl CombinedAstLints { - pub fn from_config(config: Vec<(AstLint, LintLevel)>) -> Self { - let mut combined_ast_lints = Self::default(); + impl<'compilation> CombinedAstLints<'compilation> { + pub fn from_config(config: Vec<(AstLint, LintLevel)>, compilation: Compilation<'compilation>) -> Self { + let mut combined_ast_lints = Self::new(compilation); for (lint, level) in config { match lint { $(AstLint::$lint_name => combined_ast_lints.$lint_name.level = level),* @@ -190,26 +233,26 @@ macro_rules! declare_ast_lints { combined_ast_lints } - fn check_package(&mut self, package: &Package) { $(self.$lint_name.check_package(package, &mut self.buffer));*; } - fn check_namespace(&mut self, namespace: &Namespace) { $(self.$lint_name.check_namespace(namespace, &mut self.buffer));*; } - fn check_item(&mut self, item: &Item) { $(self.$lint_name.check_item(item, &mut self.buffer));*; } - fn check_attr(&mut self, attr: &Attr) { $(self.$lint_name.check_attr(attr, &mut self.buffer));*; } - fn check_ty_def(&mut self, def: &TyDef) { $(self.$lint_name.check_ty_def(def, &mut self.buffer));*; } - fn check_callable_decl(&mut self, decl: &CallableDecl) { $(self.$lint_name.check_callable_decl(decl, &mut self.buffer));*; } - fn check_spec_decl(&mut self, decl: &SpecDecl) { $(self.$lint_name.check_spec_decl(decl, &mut self.buffer));*; } - fn check_functor_expr(&mut self, expr: &FunctorExpr) { $(self.$lint_name.check_functor_expr(expr, &mut self.buffer));*; } - fn check_ty(&mut self, ty: &Ty) { $(self.$lint_name.check_ty(ty, &mut self.buffer));*; } - fn check_block(&mut self, block: &Block) { $(self.$lint_name.check_block(block, &mut self.buffer));*; } - fn check_stmt(&mut self, stmt: &Stmt) { $(self.$lint_name.check_stmt(stmt, &mut self.buffer));*; } - fn check_expr(&mut self, expr: &Expr) { $(self.$lint_name.check_expr(expr, &mut self.buffer));*; } - fn check_pat(&mut self, pat: &Pat) { $(self.$lint_name.check_pat(pat, &mut self.buffer));*; } - fn check_qubit_init(&mut self, init: &QubitInit) { $(self.$lint_name.check_qubit_init(init, &mut self.buffer));*; } - fn check_path(&mut self, path: &Path) { $(self.$lint_name.check_path(path, &mut self.buffer));*; } - fn check_path_kind(&mut self, path: &PathKind) { $(self.$lint_name.check_path_kind(path, &mut self.buffer));*; } - fn check_ident(&mut self, ident: &Ident) { $(self.$lint_name.check_ident(ident, &mut self.buffer));*; } + fn check_package(&mut self, package: &Package) { $(self.$lint_name.check_package(package, &mut self.buffer, self.compilation));*; } + fn check_namespace(&mut self, namespace: &Namespace) { $(self.$lint_name.check_namespace(namespace, &mut self.buffer, self.compilation));*; } + fn check_item(&mut self, item: &Item) { $(self.$lint_name.check_item(item, &mut self.buffer, self.compilation));*; } + fn check_attr(&mut self, attr: &Attr) { $(self.$lint_name.check_attr(attr, &mut self.buffer, self.compilation));*; } + fn check_ty_def(&mut self, def: &TyDef) { $(self.$lint_name.check_ty_def(def, &mut self.buffer, self.compilation));*; } + fn check_callable_decl(&mut self, decl: &CallableDecl) { $(self.$lint_name.check_callable_decl(decl, &mut self.buffer, self.compilation));*; } + fn check_spec_decl(&mut self, decl: &SpecDecl) { $(self.$lint_name.check_spec_decl(decl, &mut self.buffer, self.compilation));*; } + fn check_functor_expr(&mut self, expr: &FunctorExpr) { $(self.$lint_name.check_functor_expr(expr, &mut self.buffer, self.compilation));*; } + fn check_ty(&mut self, ty: &Ty) { $(self.$lint_name.check_ty(ty, &mut self.buffer, self.compilation));*; } + fn check_block(&mut self, block: &Block) { $(self.$lint_name.check_block(block, &mut self.buffer, self.compilation));*; } + fn check_stmt(&mut self, stmt: &Stmt) { $(self.$lint_name.check_stmt(stmt, &mut self.buffer, self.compilation));*; } + fn check_expr(&mut self, expr: &Expr) { $(self.$lint_name.check_expr(expr, &mut self.buffer, self.compilation));*; } + fn check_pat(&mut self, pat: &Pat) { $(self.$lint_name.check_pat(pat, &mut self.buffer, self.compilation));*; } + fn check_qubit_init(&mut self, init: &QubitInit) { $(self.$lint_name.check_qubit_init(init, &mut self.buffer, self.compilation));*; } + fn check_path(&mut self, path: &Path) { $(self.$lint_name.check_path(path, &mut self.buffer, self.compilation));*; } + fn check_path_kind(&mut self, path: &PathKind) { $(self.$lint_name.check_path_kind(path, &mut self.buffer, self.compilation));*; } + fn check_ident(&mut self, ident: &Ident) { $(self.$lint_name.check_ident(ident, &mut self.buffer, self.compilation));*; } } - impl<'a> Visitor<'a> for CombinedAstLints { + impl<'a> Visitor<'a> for CombinedAstLints<'_> { fn visit_package(&mut self, package: &'a Package) { self.check_package(package); visit::walk_package(self, package); @@ -299,4 +342,4 @@ macro_rules! declare_ast_lints { pub(crate) use declare_ast_lints; -use super::LintKind; +use super::{Compilation, LintKind}; diff --git a/compiler/qsc_linter/src/lints/ast.rs b/compiler/qsc_linter/src/lints/ast.rs index f78035a5bc..f0a1cb2d24 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -2,7 +2,7 @@ // Licensed under the MIT License. use super::lint; -use crate::linter::ast::declare_ast_lints; +use crate::linter::{ast::declare_ast_lints, Compilation}; use qsc_ast::ast::{BinOp, Block, Expr, ExprKind, Item, ItemKind, Lit, Stmt, StmtKind}; use qsc_data_structures::span::Span; @@ -28,10 +28,11 @@ declare_ast_lints! { (NeedlessParens, LintLevel::Allow, "unnecessary parentheses", "remove the extra parentheses for clarity"), (RedundantSemicolons, LintLevel::Warn, "redundant semicolons", "remove the redundant semicolons"), (DeprecatedNewtype, LintLevel::Allow, "deprecated `newtype` declarations", "`newtype` declarations are deprecated, use `struct` instead"), + (DeprecatedSet, LintLevel::Warn, "deprecated use of `set` keyword", "`set` keywords are deprecated for assignments, they can be removed"), } impl AstLintPass for DivisionByZero { - fn check_expr(&self, expr: &Expr, buffer: &mut Vec) { + fn check_expr(&self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { if let ExprKind::BinOp(BinOp::Div, _, ref rhs) = *expr.kind { if let ExprKind::Lit(ref lit) = *rhs.kind { if let Lit::Int(0) = **lit { @@ -82,7 +83,7 @@ impl NeedlessParens { } impl AstLintPass for NeedlessParens { - fn check_expr(&self, expr: &Expr, buffer: &mut Vec) { + fn check_expr(&self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { match &*expr.kind { ExprKind::BinOp(_, left, right) => { self.push(expr, left, buffer); @@ -96,7 +97,7 @@ impl AstLintPass for NeedlessParens { } /// Checks the assignment statements. - fn check_stmt(&self, stmt: &Stmt, buffer: &mut Vec) { + fn check_stmt(&self, stmt: &Stmt, buffer: &mut Vec, _compilation: Compilation) { if let StmtKind::Local(_, _, right) = &*stmt.kind { if let ExprKind::Paren(_) = &*right.kind { buffer.push(lint!( @@ -124,7 +125,7 @@ impl AstLintPass for RedundantSemicolons { /// semicolon is parsed as an Empty statement. If we have multiple empty /// statements in a row, we group them as single lint, that spans from /// the first redundant semicolon to the last redundant semicolon. - fn check_block(&self, block: &Block, buffer: &mut Vec) { + fn check_block(&self, block: &Block, buffer: &mut Vec, _compilation: Compilation) { // a finite state machine that keeps track of the span of the redundant semicolons // None: no redundant semicolons // Some(_): one or more redundant semicolons @@ -166,9 +167,34 @@ fn precedence(expr: &Expr) -> u8 { /// Creates a lint for deprecated user-defined types declarations using `newtype`. impl AstLintPass for DeprecatedNewtype { - fn check_item(&self, item: &Item, buffer: &mut Vec) { + fn check_item(&self, item: &Item, buffer: &mut Vec, _compilation: Compilation) { if let ItemKind::Ty(_, _) = item.kind.as_ref() { buffer.push(lint!(self, item.span)); } } } + +impl AstLintPass for DeprecatedSet { + fn check_expr(&self, expr: &Expr, buffer: &mut Vec, compilation: Compilation) { + // If the expression is an assignment, check if the `set` keyword is used. + match expr.kind.as_ref() { + ExprKind::Assign(_, _) + | ExprKind::AssignOp(_, _, _) + | ExprKind::AssignUpdate(_, _, _) => { + if compilation.get_source_code(expr.span).starts_with("set ") { + // If the `set` keyword is used, push a lint. + let span = Span { + lo: expr.span.lo, + hi: expr.span.lo + 3, + }; + let code_action_span = Span { + lo: span.lo, + hi: span.lo + 4, + }; + buffer.push(lint!(self, span, vec![(String::new(), code_action_span)])); + } + } + _ => {} + } + } +} diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 44afd2fa97..cdc43516d6 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -14,6 +14,32 @@ use qsc_frontend::compile::{self, CompileUnit, PackageStore, SourceMap}; use qsc_hir::hir::CallableKind; use qsc_passes::PackageType; +#[test] +fn set_keyword_lint() { + check( + &wrap_in_callable("set x = 3;", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "set", + level: Warn, + message: "deprecated use of `set` keyword", + help: "`set` keywords are deprecated for assignments, they can be removed", + code_action_edits: [ + ( + "", + Span { + lo: 71, + hi: 75, + }, + ), + ], + }, + ] + "#]], + ); +} + #[test] fn multiple_lints() { check( @@ -677,7 +703,7 @@ fn run_lints( compile_unit, }; - let mut ast_lints = run_ast_lints(&compile_unit.ast.package, config); + 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 mut lints = Vec::new(); lints.append(&mut ast_lints); diff --git a/compiler/qsc_parse/src/expr.rs b/compiler/qsc_parse/src/expr.rs index 5647d7bbcc..da2693bc18 100644 --- a/compiler/qsc_parse/src/expr.rs +++ b/compiler/qsc_parse/src/expr.rs @@ -343,29 +343,6 @@ fn expr_if(s: &mut ParserContext) -> Result> { Ok(Box::new(ExprKind::If(cond, body, otherwise))) } -fn expr_set(s: &mut ParserContext) -> Result> { - let lhs = expr(s)?; - if token(s, TokenKind::Eq).is_ok() { - let rhs = expr(s)?; - Ok(Box::new(ExprKind::Assign(lhs, rhs))) - } else if token(s, TokenKind::WSlashEq).is_ok() { - let index = expr(s)?; - token(s, TokenKind::LArrow)?; - let rhs = expr(s)?; - Ok(Box::new(ExprKind::AssignUpdate(lhs, index, rhs))) - } else if let TokenKind::BinOpEq(op) = s.peek().kind { - s.advance(); - let rhs = expr(s)?; - Ok(Box::new(ExprKind::AssignOp(closed_bin_op(op), lhs, rhs))) - } else { - Err(Error::new(ErrorKind::Rule( - "assignment operator", - s.peek().kind, - s.peek().span, - ))) - } -} - fn expr_array(s: &mut ParserContext) -> Result> { token(s, TokenKind::Open(Delim::Bracket))?; let kind = expr_array_core(s)?; From b609dc343fa9668ffcde5befb443b0ce784284b9 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Thu, 12 Dec 2024 10:07:02 -0800 Subject: [PATCH 03/12] added test --- compiler/qsc_parse/src/expr/tests.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/compiler/qsc_parse/src/expr/tests.rs b/compiler/qsc_parse/src/expr/tests.rs index 2389c5d305..8ba68aaecd 100644 --- a/compiler/qsc_parse/src/expr/tests.rs +++ b/compiler/qsc_parse/src/expr/tests.rs @@ -723,6 +723,30 @@ fn set() { ); } +#[test] +fn set_chained() { + check( + expr, + "set x = set y = z", + &expect![[r#" + Expr _id_ [0-17]: Assign: + Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "x") + Expr _id_ [8-17]: Assign: + Expr _id_ [12-13]: Path: Path _id_ [12-13] (Ident _id_ [12-13] "y") + Expr _id_ [16-17]: Path: Path _id_ [16-17] (Ident _id_ [16-17] "z")"#]], + ); + check( + expr, + "x = y = z", + &expect![[r#" + Expr _id_ [0-9]: Assign: + Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") + Expr _id_ [4-9]: Assign: + Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "y") + Expr _id_ [8-9]: Path: Path _id_ [8-9] (Ident _id_ [8-9] "z")"#]], + ); +} + #[test] fn set_hole() { check( From 2ac40c23cb95c2bd44a25e2f4bf0a2a225418145 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Thu, 12 Dec 2024 10:28:30 -0800 Subject: [PATCH 04/12] removed `set` from other code action, made code action more robust to different whitespace --- compiler/qsc_linter/src/lints/ast.rs | 11 +++++------ compiler/qsc_linter/src/lints/hir.rs | 2 +- compiler/qsc_linter/src/tests.rs | 10 +++++----- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/compiler/qsc_linter/src/lints/ast.rs b/compiler/qsc_linter/src/lints/ast.rs index f0a1cb2d24..2840770733 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -181,17 +181,16 @@ impl AstLintPass for DeprecatedSet { ExprKind::Assign(_, _) | ExprKind::AssignOp(_, _, _) | ExprKind::AssignUpdate(_, _, _) => { - if compilation.get_source_code(expr.span).starts_with("set ") { + if ["set ", "set\n", "set\r\n", "set\t"] + .iter() + .any(|s| compilation.get_source_code(expr.span).starts_with(s)) + { // If the `set` keyword is used, push a lint. let span = Span { lo: expr.span.lo, hi: expr.span.lo + 3, }; - let code_action_span = Span { - lo: span.lo, - hi: span.lo + 4, - }; - buffer.push(lint!(self, span, vec![(String::new(), code_action_span)])); + buffer.push(lint!(self, span, vec![(String::new(), span)])); } } _ => {} diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index ffbc70da13..cddb2d2019 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -222,7 +222,7 @@ impl HirLintPass for DeprecatedWithOperator { (compilation.indentation_at_offset(info.span.lo) + 4) as usize; let innermost_expr = compilation.get_source_code(expr.span); let mut new_expr = if info.is_w_eq { - format!("set {} = new {} {{\n", innermost_expr, info.ty_name) + format!("{} = new {} {{\n", innermost_expr, info.ty_name) } else { format!("new {} {{\n", info.ty_name) }; diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index cdc43516d6..1142284621 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -30,7 +30,7 @@ fn set_keyword_lint() { "", Span { lo: 71, - hi: 75, + hi: 74, }, ), ], @@ -494,23 +494,23 @@ fn deprecated_with_eq_op_for_structs() { struct Foo { x : Int } function Bar() : Foo { mutable foo = new Foo { x = 2 }; - set foo w/= x <- 3; + foo w/= x <- 3; foo } "}, &expect![[r#" [ SrcLint { - source: "set foo w/= x <- 3", + source: "foo w/= x <- 3", level: Allow, message: "deprecated `w/` and `w/=` operators for structs", help: "`w/` and `w/=` operators for structs are deprecated, use `new` instead", code_action_edits: [ ( - "set foo = new Foo {\n ...foo,\n x = 3,\n }", + "foo = new Foo {\n ...foo,\n x = 3,\n }", Span { lo: 115, - hi: 133, + hi: 129, }, ), ], From 5a60c401b46f5795c78281ee7b8e5f9a84d8e172 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Thu, 12 Dec 2024 15:19:39 -0800 Subject: [PATCH 05/12] change lint level to `Allow` --- compiler/qsc_linter/src/lints/ast.rs | 2 +- samples/algorithms/MajoranaQubits/src/Main.qs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/qsc_linter/src/lints/ast.rs b/compiler/qsc_linter/src/lints/ast.rs index 2840770733..ff4b23ef46 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -28,7 +28,7 @@ declare_ast_lints! { (NeedlessParens, LintLevel::Allow, "unnecessary parentheses", "remove the extra parentheses for clarity"), (RedundantSemicolons, LintLevel::Warn, "redundant semicolons", "remove the redundant semicolons"), (DeprecatedNewtype, LintLevel::Allow, "deprecated `newtype` declarations", "`newtype` declarations are deprecated, use `struct` instead"), - (DeprecatedSet, LintLevel::Warn, "deprecated use of `set` keyword", "`set` keywords are deprecated for assignments, they can be removed"), + (DeprecatedSet, LintLevel::Allow, "deprecated use of `set` keyword", "`set` keywords are deprecated for assignments, they can be removed"), } impl AstLintPass for DivisionByZero { diff --git a/samples/algorithms/MajoranaQubits/src/Main.qs b/samples/algorithms/MajoranaQubits/src/Main.qs index 4047d7c931..71d56806a3 100644 --- a/samples/algorithms/MajoranaQubits/src/Main.qs +++ b/samples/algorithms/MajoranaQubits/src/Main.qs @@ -4,7 +4,7 @@ /// # Description /// In hardware providing majorana qubits, common quantum operations /// are implemented using measurements and Pauli corrections. This -/// sample shows a hypotetical hardware provider exposing some custom +/// sample shows a hypothetical hardware provider exposing some custom /// gates to Q# and a small library built on top of it. /// Sample program using custom gates from a hardware provider. From 1a64084bc5d7b6a4782e39ac5133bcb5f55ac516 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Thu, 12 Dec 2024 16:02:38 -0800 Subject: [PATCH 06/12] update test --- compiler/qsc_linter/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 1142284621..edc8171314 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -22,7 +22,7 @@ fn set_keyword_lint() { [ SrcLint { source: "set", - level: Warn, + level: Allow, message: "deprecated use of `set` keyword", help: "`set` keywords are deprecated for assignments, they can be removed", code_action_edits: [ From 3f970f4064c81aa1fd687bbdce172c678dd8492c Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Thu, 12 Dec 2024 16:23:35 -0800 Subject: [PATCH 07/12] tie `set` to v2-preview-syntax flag --- compiler/qsc_parse/src/expr.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/qsc_parse/src/expr.rs b/compiler/qsc_parse/src/expr.rs index da2693bc18..215da85c13 100644 --- a/compiler/qsc_parse/src/expr.rs +++ b/compiler/qsc_parse/src/expr.rs @@ -27,7 +27,7 @@ use qsc_ast::ast::{ self, BinOp, CallableKind, Expr, ExprKind, FieldAccess, FieldAssign, Functor, Lit, NodeId, Pat, PatKind, PathKind, Pauli, StringComponent, TernOp, UnOp, }; -use qsc_data_structures::span::Span; +use qsc_data_structures::{language_features::LanguageFeatures, span::Span}; use std::{result, str::FromStr}; struct PrefixOp { @@ -228,7 +228,9 @@ fn expr_base(s: &mut ParserContext) -> Result> { Ok(Box::new(ExprKind::Repeat(body, cond, fixup))) } else if token(s, TokenKind::Keyword(Keyword::Return)).is_ok() { Ok(Box::new(ExprKind::Return(expr(s)?))) - } else if token(s, TokenKind::Keyword(Keyword::Set)).is_ok() { + } else if !s.contains_language_feature(LanguageFeatures::V2PreviewSyntax) + && token(s, TokenKind::Keyword(Keyword::Set)).is_ok() + { // Need to rewrite the span of the expr to include the `set` keyword. return expr(s).map(|assign| { Box::new(Expr { From 4af7e35c5707cf84b5ab9eaffd13f1783dc88299 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Fri, 13 Dec 2024 10:26:47 -0800 Subject: [PATCH 08/12] remove dev test --- compiler/qsc_parse/src/expr/tests.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/compiler/qsc_parse/src/expr/tests.rs b/compiler/qsc_parse/src/expr/tests.rs index 8ba68aaecd..562c4d20fe 100644 --- a/compiler/qsc_parse/src/expr/tests.rs +++ b/compiler/qsc_parse/src/expr/tests.rs @@ -5,20 +5,6 @@ use super::expr; use crate::tests::check; use expect_test::expect; -#[test] -fn foo() { - check( - expr, - "x = j = 3", - &expect![[r#" - Expr _id_ [0-9]: Assign: - Expr _id_ [0-1]: Path: Path _id_ [0-1] (Ident _id_ [0-1] "x") - Expr _id_ [4-9]: Assign: - Expr _id_ [4-5]: Path: Path _id_ [4-5] (Ident _id_ [4-5] "j") - Expr _id_ [8-9]: Lit: Int(3)"#]], - ); -} - #[test] fn lit_int() { check(expr, "123", &expect!["Expr _id_ [0-3]: Lit: Int(123)"]); From 70523253fd3f1f35018fe616819a6e46282140b4 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Fri, 13 Dec 2024 16:12:34 -0800 Subject: [PATCH 09/12] add lint options for qsharp.json --- vscode/qsharp.schema.json | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vscode/qsharp.schema.json b/vscode/qsharp.schema.json index 34de82d379..6999eb87c5 100644 --- a/vscode/qsharp.schema.json +++ b/vscode/qsharp.schema.json @@ -34,9 +34,12 @@ "divisionByZero", "needlessParens", "redundantSemicolons", + "deprecatedNewtype", + "deprecatedSet", + "needlessOperation", + "deprecatedFunctionConstructor", "deprecatedWithOperator", - "deprecatedDoubleColonOperator", - "deprecatedNewtype" + "deprecatedDoubleColonOperator" ] }, "level": { From 9b647cbf0934a3bf608c0098b5f81563766717de Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Fri, 13 Dec 2024 16:13:15 -0800 Subject: [PATCH 10/12] discourage daisy chain lint --- compiler/qsc_linter/src/linter/ast.rs | 46 ++++++------ compiler/qsc_linter/src/lints/ast.rs | 102 +++++++++++++++++++++----- compiler/qsc_linter/src/tests.rs | 61 +++++++++++++++ vscode/qsharp.schema.json | 1 + 4 files changed, 167 insertions(+), 43 deletions(-) diff --git a/compiler/qsc_linter/src/linter/ast.rs b/compiler/qsc_linter/src/linter/ast.rs index e6610909e2..b9a201ba33 100644 --- a/compiler/qsc_linter/src/linter/ast.rs +++ b/compiler/qsc_linter/src/linter/ast.rs @@ -50,65 +50,71 @@ pub fn run_ast_lints( /// The trait provides default empty implementations for the rest of the methods, /// which will be optimized to a no-op by the rust compiler. pub(crate) trait AstLintPass { - fn check_attr(&self, _attr: &Attr, _buffer: &mut Vec, _compilation: Compilation) {} - fn check_block(&self, _block: &Block, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_attr(&mut self, _attr: &Attr, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_block(&mut self, _block: &Block, _buffer: &mut Vec, _compilation: Compilation) {} fn check_callable_decl( - &self, + &mut self, _callable_decl: &CallableDecl, _buffer: &mut Vec, _compilation: Compilation, ) { } - fn check_expr(&self, _expr: &Expr, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_expr(&mut self, _expr: &Expr, _buffer: &mut Vec, _compilation: Compilation) {} fn check_functor_expr( - &self, + &mut self, _functor_expr: &FunctorExpr, _buffer: &mut Vec, _compilation: Compilation, ) { } - fn check_ident(&self, _ident: &Ident, _buffer: &mut Vec, _compilation: Compilation) {} - fn check_item(&self, _item: &Item, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_ident(&mut self, _ident: &Ident, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_item(&mut self, _item: &Item, _buffer: &mut Vec, _compilation: Compilation) {} fn check_namespace( - &self, + &mut self, _namespace: &Namespace, _buffer: &mut Vec, _compilation: Compilation, ) { } fn check_package( - &self, + &mut self, _package: &Package, _buffer: &mut Vec, _compilation: Compilation, ) { } - fn check_pat(&self, _pat: &Pat, _buffer: &mut Vec, _compilation: Compilation) {} - fn check_path(&self, _path: &Path, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_pat(&mut self, _pat: &Pat, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_path(&mut self, _path: &Path, _buffer: &mut Vec, _compilation: Compilation) {} fn check_path_kind( - &self, + &mut self, _path: &PathKind, _buffer: &mut Vec, _compilation: Compilation, ) { } fn check_qubit_init( - &self, + &mut self, _qubit_init: &QubitInit, _buffer: &mut Vec, _compilation: Compilation, ) { } fn check_spec_decl( - &self, + &mut self, _spec_decl: &SpecDecl, _buffer: &mut Vec, _compilation: Compilation, ) { } - fn check_stmt(&self, _stmt: &Stmt, _buffer: &mut Vec, _compilation: Compilation) {} - fn check_ty(&self, _ty: &Ty, _buffer: &mut Vec, _compilation: Compilation) {} - fn check_ty_def(&self, _ty_def: &TyDef, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_stmt(&mut self, _stmt: &Stmt, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_ty(&mut self, _ty: &Ty, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_ty_def( + &mut self, + _ty_def: &TyDef, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } } /// This macro allow us to declare lints while avoiding boilerplate. It does three things: @@ -152,12 +158,6 @@ macro_rules! declare_ast_lints { // Declare & implement a struct representing a lint. (@LINT_STRUCT $lint_name:ident, $default_level:expr, $msg:expr, $help:expr) => { - - #[derive(Default)] - pub(crate) struct $lint_name { - level: LintLevel, - } - impl $lint_name { const DEFAULT_LEVEL: LintLevel = $default_level; diff --git a/compiler/qsc_linter/src/lints/ast.rs b/compiler/qsc_linter/src/lints/ast.rs index ff4b23ef46..d3ddfb0da3 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -3,7 +3,7 @@ use super::lint; use crate::linter::{ast::declare_ast_lints, Compilation}; -use qsc_ast::ast::{BinOp, Block, Expr, ExprKind, Item, ItemKind, Lit, Stmt, StmtKind}; +use qsc_ast::ast::{BinOp, Block, Expr, ExprKind, Item, ItemKind, Lit, NodeId, Stmt, StmtKind}; use qsc_data_structures::span::Span; // Read Me: @@ -29,10 +29,16 @@ declare_ast_lints! { (RedundantSemicolons, LintLevel::Warn, "redundant semicolons", "remove the redundant semicolons"), (DeprecatedNewtype, LintLevel::Allow, "deprecated `newtype` declarations", "`newtype` declarations are deprecated, use `struct` instead"), (DeprecatedSet, LintLevel::Allow, "deprecated use of `set` keyword", "`set` keywords are deprecated for assignments, they can be removed"), + (DiscourageDaisyChain, LintLevel::Warn, "discouraged use of chain assignment", "assignment expressions always return `Unit`, so chaining them may not be useful"), +} + +#[derive(Default)] +struct DivisionByZero { + level: LintLevel, } impl AstLintPass for DivisionByZero { - fn check_expr(&self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { if let ExprKind::BinOp(BinOp::Div, _, ref rhs) = *expr.kind { if let ExprKind::Lit(ref lit) = *rhs.kind { if let Lit::Int(0) = **lit { @@ -43,6 +49,11 @@ impl AstLintPass for DivisionByZero { } } +#[derive(Default)] +struct NeedlessParens { + level: LintLevel, +} + impl NeedlessParens { /// The idea is that if we find a expr of the form: /// a + (expr) @@ -83,7 +94,7 @@ impl NeedlessParens { } impl AstLintPass for NeedlessParens { - fn check_expr(&self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { match &*expr.kind { ExprKind::BinOp(_, left, right) => { self.push(expr, left, buffer); @@ -97,7 +108,7 @@ impl AstLintPass for NeedlessParens { } /// Checks the assignment statements. - fn check_stmt(&self, stmt: &Stmt, buffer: &mut Vec, _compilation: Compilation) { + fn check_stmt(&mut self, stmt: &Stmt, buffer: &mut Vec, _compilation: Compilation) { if let StmtKind::Local(_, _, right) = &*stmt.kind { if let ExprKind::Paren(_) = &*right.kind { buffer.push(lint!( @@ -110,6 +121,11 @@ impl AstLintPass for NeedlessParens { } } +#[derive(Default)] +struct RedundantSemicolons { + level: LintLevel, +} + impl RedundantSemicolons { /// Helper function that pushes a lint to the buffer if we have /// found two or more semicolons. @@ -125,7 +141,7 @@ impl AstLintPass for RedundantSemicolons { /// semicolon is parsed as an Empty statement. If we have multiple empty /// statements in a row, we group them as single lint, that spans from /// the first redundant semicolon to the last redundant semicolon. - fn check_block(&self, block: &Block, buffer: &mut Vec, _compilation: Compilation) { + fn check_block(&mut self, block: &Block, buffer: &mut Vec, _compilation: Compilation) { // a finite state machine that keeps track of the span of the redundant semicolons // None: no redundant semicolons // Some(_): one or more redundant semicolons @@ -165,32 +181,78 @@ fn precedence(expr: &Expr) -> u8 { } } +#[derive(Default)] +struct DeprecatedNewtype { + level: LintLevel, +} + /// Creates a lint for deprecated user-defined types declarations using `newtype`. impl AstLintPass for DeprecatedNewtype { - fn check_item(&self, item: &Item, buffer: &mut Vec, _compilation: Compilation) { + fn check_item(&mut self, item: &Item, buffer: &mut Vec, _compilation: Compilation) { if let ItemKind::Ty(_, _) = item.kind.as_ref() { buffer.push(lint!(self, item.span)); } } } +fn is_assign(expr: &Expr) -> bool { + matches!( + expr.kind.as_ref(), + ExprKind::Assign(_, _) | ExprKind::AssignOp(_, _, _) | ExprKind::AssignUpdate(_, _, _) + ) +} + +#[derive(Default)] +struct DeprecatedSet { + level: LintLevel, +} + impl AstLintPass for DeprecatedSet { - fn check_expr(&self, expr: &Expr, buffer: &mut Vec, compilation: Compilation) { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, compilation: Compilation) { // If the expression is an assignment, check if the `set` keyword is used. + if is_assign(expr) + && ["set ", "set\n", "set\r\n", "set\t"] + .iter() + .any(|s| compilation.get_source_code(expr.span).starts_with(s)) + { + // If the `set` keyword is used, push a lint. + let span = Span { + lo: expr.span.lo, + hi: expr.span.lo + 3, + }; + buffer.push(lint!(self, span, vec![(String::new(), span)])); + } + } +} + +#[derive(Default)] +struct DiscourageDaisyChain { + level: LintLevel, + // Keeps track of the expressions that won't create lints because they are part of a chain. + repressed_exprs: Vec, +} + +impl AstLintPass for DiscourageDaisyChain { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { match expr.kind.as_ref() { - ExprKind::Assign(_, _) - | ExprKind::AssignOp(_, _, _) - | ExprKind::AssignUpdate(_, _, _) => { - if ["set ", "set\n", "set\r\n", "set\t"] - .iter() - .any(|s| compilation.get_source_code(expr.span).starts_with(s)) - { - // If the `set` keyword is used, push a lint. - let span = Span { - lo: expr.span.lo, - hi: expr.span.lo + 3, - }; - buffer.push(lint!(self, span, vec![(String::new(), span)])); + ExprKind::Assign(_, rhs) + | ExprKind::AssignOp(_, _, rhs) + | ExprKind::AssignUpdate(_, _, rhs) => { + // Check to see if this expressions is part of an existing chain. + let should_repress = self + .repressed_exprs + .last() + .map_or_else(|| false, |l| *l == expr.id); + if should_repress { + self.repressed_exprs.pop(); + } + if is_assign(rhs) { + // If this is a new chain, push a lint. + if !should_repress { + buffer.push(lint!(self, expr.span)); + } + // Add the rhs to the repressed expressions because it is now part of an existing chain. + self.repressed_exprs.push(rhs.id); } } _ => {} diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index edc8171314..a4cee6b8c8 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -14,6 +14,67 @@ use qsc_frontend::compile::{self, CompileUnit, PackageStore, SourceMap}; use qsc_hir::hir::CallableKind; use qsc_passes::PackageType; +#[test] +fn daisy_chain_lint() { + check( + &wrap_in_callable("x = y = z", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "x = y = z", + level: Warn, + message: "discouraged use of chain assignment", + help: "assignment expressions always return `Unit`, so chaining them may not be useful", + code_action_edits: [], + }, + ] + "#]], + ); +} + +#[test] +fn long_daisy_chain_lint() { + check( + &wrap_in_callable("x = y = z = z = z", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "x = y = z = z = z", + level: Warn, + message: "discouraged use of chain assignment", + help: "assignment expressions always return `Unit`, so chaining them may not be useful", + code_action_edits: [], + }, + ] + "#]], + ); +} + +#[test] +fn nested_daisy_chain_lint() { + check( + &wrap_in_callable("x = y = { a = b = c; z } = z = z", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "x = y = { a = b = c; z } = z = z", + level: Warn, + message: "discouraged use of chain assignment", + help: "assignment expressions always return `Unit`, so chaining them may not be useful", + code_action_edits: [], + }, + SrcLint { + source: "a = b = c", + level: Warn, + message: "discouraged use of chain assignment", + help: "assignment expressions always return `Unit`, so chaining them may not be useful", + code_action_edits: [], + }, + ] + "#]], + ); +} + #[test] fn set_keyword_lint() { check( diff --git a/vscode/qsharp.schema.json b/vscode/qsharp.schema.json index 6999eb87c5..7f81cd7f0e 100644 --- a/vscode/qsharp.schema.json +++ b/vscode/qsharp.schema.json @@ -36,6 +36,7 @@ "redundantSemicolons", "deprecatedNewtype", "deprecatedSet", + "discourageDaisyChain", "needlessOperation", "deprecatedFunctionConstructor", "deprecatedWithOperator", From 0c2c5afa4964e50f32bf966030957e143ae1e32b Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Fri, 13 Dec 2024 16:33:54 -0800 Subject: [PATCH 11/12] updated wording of lint --- compiler/qsc_linter/src/lints/ast.rs | 2 +- compiler/qsc_linter/src/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/qsc_linter/src/lints/ast.rs b/compiler/qsc_linter/src/lints/ast.rs index d3ddfb0da3..578fb11525 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -28,7 +28,7 @@ declare_ast_lints! { (NeedlessParens, LintLevel::Allow, "unnecessary parentheses", "remove the extra parentheses for clarity"), (RedundantSemicolons, LintLevel::Warn, "redundant semicolons", "remove the redundant semicolons"), (DeprecatedNewtype, LintLevel::Allow, "deprecated `newtype` declarations", "`newtype` declarations are deprecated, use `struct` instead"), - (DeprecatedSet, LintLevel::Allow, "deprecated use of `set` keyword", "`set` keywords are deprecated for assignments, they can be removed"), + (DeprecatedSet, LintLevel::Allow, "deprecated use of `set` keyword", "the `set` keyword is deprecated for assignments and can be removed"), (DiscourageDaisyChain, LintLevel::Warn, "discouraged use of chain assignment", "assignment expressions always return `Unit`, so chaining them may not be useful"), } diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index a4cee6b8c8..b9a014bf34 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -85,7 +85,7 @@ fn set_keyword_lint() { source: "set", level: Allow, message: "deprecated use of `set` keyword", - help: "`set` keywords are deprecated for assignments, they can be removed", + help: "the `set` keyword is deprecated for assignments and can be removed", code_action_edits: [ ( "", From 23b1ca688a8b491007cabfb36670c130e3f7a806 Mon Sep 17 00:00:00 2001 From: Scott Carda Date: Mon, 16 Dec 2024 13:12:20 -0800 Subject: [PATCH 12/12] renamed lint to `DiscourageChainAssignment` --- compiler/qsc_linter/src/lints/ast.rs | 6 +++--- vscode/qsharp.schema.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/qsc_linter/src/lints/ast.rs b/compiler/qsc_linter/src/lints/ast.rs index 578fb11525..72bdf16a57 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -29,7 +29,7 @@ declare_ast_lints! { (RedundantSemicolons, LintLevel::Warn, "redundant semicolons", "remove the redundant semicolons"), (DeprecatedNewtype, LintLevel::Allow, "deprecated `newtype` declarations", "`newtype` declarations are deprecated, use `struct` instead"), (DeprecatedSet, LintLevel::Allow, "deprecated use of `set` keyword", "the `set` keyword is deprecated for assignments and can be removed"), - (DiscourageDaisyChain, LintLevel::Warn, "discouraged use of chain assignment", "assignment expressions always return `Unit`, so chaining them may not be useful"), + (DiscourageChainAssignment, LintLevel::Warn, "discouraged use of chain assignment", "assignment expressions always return `Unit`, so chaining them may not be useful"), } #[derive(Default)] @@ -226,13 +226,13 @@ impl AstLintPass for DeprecatedSet { } #[derive(Default)] -struct DiscourageDaisyChain { +struct DiscourageChainAssignment { level: LintLevel, // Keeps track of the expressions that won't create lints because they are part of a chain. repressed_exprs: Vec, } -impl AstLintPass for DiscourageDaisyChain { +impl AstLintPass for DiscourageChainAssignment { fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { match expr.kind.as_ref() { ExprKind::Assign(_, rhs) diff --git a/vscode/qsharp.schema.json b/vscode/qsharp.schema.json index 7f81cd7f0e..ffb22cfe41 100644 --- a/vscode/qsharp.schema.json +++ b/vscode/qsharp.schema.json @@ -36,7 +36,7 @@ "redundantSemicolons", "deprecatedNewtype", "deprecatedSet", - "discourageDaisyChain", + "discourageChainAssignment", "needlessOperation", "deprecatedFunctionConstructor", "deprecatedWithOperator",