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..b9a201ba33 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,71 @@ 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(&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( + &mut self, + _callable_decl: &CallableDecl, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_expr(&mut self, _expr: &Expr, _buffer: &mut Vec, _compilation: Compilation) {} + fn check_functor_expr( + &mut self, + _functor_expr: &FunctorExpr, + _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( + &mut self, + _namespace: &Namespace, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_package( + &mut self, + _package: &Package, + _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( + &mut self, + _path: &PathKind, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_qubit_init( + &mut self, + _qubit_init: &QubitInit, + _buffer: &mut Vec, + _compilation: Compilation, + ) { + } + fn check_spec_decl( + &mut self, + _spec_decl: &SpecDecl, + _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: @@ -82,7 +134,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 +158,14 @@ macro_rules! declare_ast_lints { // Declare & implement a struct representing a lint. (@LINT_STRUCT $lint_name:ident, $default_level:expr, $msg:expr, $help:expr) => { - 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..72bdf16a57 100644 --- a/compiler/qsc_linter/src/lints/ast.rs +++ b/compiler/qsc_linter/src/lints/ast.rs @@ -2,8 +2,8 @@ // Licensed under the MIT License. use super::lint; -use crate::linter::ast::declare_ast_lints; -use qsc_ast::ast::{BinOp, Block, Expr, ExprKind, Item, ItemKind, Lit, Stmt, StmtKind}; +use crate::linter::{ast::declare_ast_lints, Compilation}; +use qsc_ast::ast::{BinOp, Block, Expr, ExprKind, Item, ItemKind, Lit, NodeId, Stmt, StmtKind}; use qsc_data_structures::span::Span; // Read Me: @@ -28,10 +28,17 @@ 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", "the `set` keyword is deprecated for assignments and can be removed"), + (DiscourageChainAssignment, 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) { + 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 { @@ -42,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) @@ -82,7 +94,7 @@ impl NeedlessParens { } impl AstLintPass for NeedlessParens { - fn check_expr(&self, expr: &Expr, buffer: &mut Vec) { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { match &*expr.kind { ExprKind::BinOp(_, left, right) => { self.push(expr, left, buffer); @@ -96,7 +108,7 @@ impl AstLintPass for NeedlessParens { } /// Checks the assignment statements. - fn check_stmt(&self, stmt: &Stmt, buffer: &mut Vec) { + 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!( @@ -109,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. @@ -124,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) { + 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 @@ -164,11 +181,81 @@ 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) { + 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(&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 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 DiscourageChainAssignment { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { + match expr.kind.as_ref() { + 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/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 44afd2fa97..b9a014bf34 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -14,6 +14,93 @@ 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( + &wrap_in_callable("set x = 3;", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "set", + level: Allow, + message: "deprecated use of `set` keyword", + help: "the `set` keyword is deprecated for assignments and can be removed", + code_action_edits: [ + ( + "", + Span { + lo: 71, + hi: 74, + }, + ), + ], + }, + ] + "#]], + ); +} + #[test] fn multiple_lints() { check( @@ -468,23 +555,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, }, ), ], @@ -677,7 +764,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 433dcf2d55..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 { @@ -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))?; @@ -211,8 +228,20 @@ 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() { - expr_set(s) + } 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 { + 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() { @@ -316,29 +345,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)?; @@ -357,8 +363,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 +375,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 +588,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..562c4d20fe 100644 --- a/compiler/qsc_parse/src/expr/tests.rs +++ b/compiler/qsc_parse/src/expr/tests.rs @@ -699,6 +699,38 @@ 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] +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] @@ -711,6 +743,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 +767,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 +799,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 +827,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 +847,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 +867,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 +887,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 +907,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 +927,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 +947,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 +967,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 +987,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 +1007,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 +1027,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 +1047,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 +1067,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 +1088,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 +1254,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 +1267,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 +2661,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 +2712,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] 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. diff --git a/vscode/qsharp.schema.json b/vscode/qsharp.schema.json index 34de82d379..ffb22cfe41 100644 --- a/vscode/qsharp.schema.json +++ b/vscode/qsharp.schema.json @@ -34,9 +34,13 @@ "divisionByZero", "needlessParens", "redundantSemicolons", + "deprecatedNewtype", + "deprecatedSet", + "discourageChainAssignment", + "needlessOperation", + "deprecatedFunctionConstructor", "deprecatedWithOperator", - "deprecatedDoubleColonOperator", - "deprecatedNewtype" + "deprecatedDoubleColonOperator" ] }, "level": {