diff --git a/Cargo.lock b/Cargo.lock index 1d42676d16..364091a150 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,6 +1104,7 @@ name = "qsc_linter" version = "0.0.0" dependencies = [ "expect-test", + "indoc", "miette", "qsc", "qsc_ast", diff --git a/compiler/qsc_linter/Cargo.toml b/compiler/qsc_linter/Cargo.toml index f6843ff843..4027ac94f2 100644 --- a/compiler/qsc_linter/Cargo.toml +++ b/compiler/qsc_linter/Cargo.toml @@ -19,6 +19,7 @@ thiserror = { workspace = true } [dev-dependencies] expect-test = { workspace = true } +indoc = { workspace = true } qsc_parse = { path = "../qsc_parse" } serde_json = { workspace = true } qsc = { path = "../qsc" } diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 3cf99f9ffd..55dac75813 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -1,17 +1,106 @@ -use qsc_hir::hir::Lit; +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use qsc_hir::{ + hir::{CallableDecl, CallableKind, Expr, ExprKind, SpecBody, SpecDecl, Stmt, StmtKind}, + ty::Ty, + visit::{self, Visitor}, +}; use crate::linter::hir::declare_hir_lints; use super::lint; declare_hir_lints! { - (Placeholder, LintLevel::Allow, "this a placeholder", "remove after addding the first HIR lint"), + (NeedlessOperation, LintLevel::Warn, "operation does not contain any quantum operations", "this callable can be declared as a function instead") +} + +/// Helper to check if an operation has desired operation characteristics +/// +/// empty operations: no lint, special case of `I` operation used for Delay +/// operations with errors (e.g. partially typed code): no lint because linter does not run +/// non-empty operations, with specializations, and no quantum operations: show lint, but don't offer quickfix (to avoid deleting user code in any explicit specializations) +/// non-empty operations with no specializations, and no quantum operations: show lint, offer quickfix to convert to function +#[derive(Default)] +struct IsQuantumOperation { + /// This field is set to `true` after calling `Visitor::visit_callable_decl(...)` + /// if the operation satisfies the characteristics described above. + is_op: bool, } -impl HirLintPass for Placeholder { - fn check_expr(&self, expr: &qsc_hir::hir::Expr, buffer: &mut Vec) { - if let qsc_hir::hir::ExprKind::Lit(Lit::Int(42)) = &expr.kind { - buffer.push(lint!(self, expr.span)); +impl IsQuantumOperation { + /// Returns `true` if the declaration is empty. + fn is_empty_decl(spec_decl: Option<&SpecDecl>) -> bool { + match spec_decl { + None => true, + Some(decl) => match &decl.body { + SpecBody::Gen(_) => true, + SpecBody::Impl(_, block) => block.stmts.is_empty(), + }, + } + } + + /// Returns `true` if the operation is empty. + /// An operation is empty if there is no code for its body and specializations. + fn is_empty_op(call_decl: &CallableDecl) -> bool { + Self::is_empty_decl(Some(&call_decl.body)) + && Self::is_empty_decl(call_decl.adj.as_ref()) + && Self::is_empty_decl(call_decl.ctl.as_ref()) + && Self::is_empty_decl(call_decl.ctl_adj.as_ref()) + } +} + +impl Visitor<'_> for IsQuantumOperation { + fn visit_callable_decl(&mut self, decl: &CallableDecl) { + if Self::is_empty_op(decl) { + self.is_op = true; + } else { + visit::walk_callable_decl(self, decl); + } + } + + fn visit_stmt(&mut self, stmt: &Stmt) { + if !self.is_op { + if let StmtKind::Qubit(..) = &stmt.kind { + self.is_op = true; + } else { + visit::walk_stmt(self, stmt); + } + } + } + + fn visit_expr(&mut self, expr: &Expr) { + if !self.is_op { + match &expr.kind { + ExprKind::Call(callee, _) => { + if matches!(&callee.ty, Ty::Arrow(arrow) if arrow.kind == CallableKind::Operation) + { + self.is_op = true; + } + } + ExprKind::Conjugate(..) | ExprKind::Repeat(..) => { + self.is_op = true; + } + _ => { + visit::walk_expr(self, expr); + } + } + } + } +} + +/// HIR Lint for [`NeedlessOperation`], suggesting to use function +/// We use [`IsQuantumOperation`] helper to check if a operation has desired operation characteristics +impl HirLintPass for NeedlessOperation { + fn check_callable_decl(&self, decl: &CallableDecl, buffer: &mut Vec) { + if decl.kind == CallableKind::Operation { + let mut op_limits = IsQuantumOperation::default(); + + op_limits.visit_callable_decl(decl); + + if !op_limits.is_op { + buffer.push(lint!(self, decl.name.span)); + } } } } diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 1217488905..317b4976a7 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -6,14 +6,16 @@ use crate::{ Lint, LintConfig, LintLevel, }; use expect_test::{expect, Expect}; +use indoc::indoc; use qsc_data_structures::{language_features::LanguageFeatures, target::TargetCapabilityFlags}; use qsc_frontend::compile::{self, CompileUnit, PackageStore, SourceMap}; +use qsc_hir::hir::CallableKind; use qsc_passes::PackageType; #[test] fn multiple_lints() { check( - "let x = ((1 + 2)) / 0;;;;", + &wrap_in_callable("let x = ((1 + 2)) / 0;;;;", CallableKind::Operation), &expect![[r#" [ SrcLint { @@ -34,6 +36,12 @@ fn multiple_lints() { message: "unnecessary parentheses", help: "remove the extra parentheses for clarity", }, + SrcLint { + source: "RunProgram", + level: Warn, + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", + }, ] "#]], ); @@ -42,7 +50,7 @@ fn multiple_lints() { #[test] fn double_parens() { check( - "let x = ((1 + 2));", + &wrap_in_callable("let x = ((1 + 2));", CallableKind::Function), &expect![[r#" [ SrcLint { @@ -59,7 +67,7 @@ fn double_parens() { #[test] fn division_by_zero() { check( - "let x = 2 / 0;", + &wrap_in_callable("let x = 2 / 0;", CallableKind::Function), &expect![[r#" [ SrcLint { @@ -76,7 +84,7 @@ fn division_by_zero() { #[test] fn needless_parens_in_assignment() { check( - "let x = (42);", + &wrap_in_callable("let x = (42);", CallableKind::Function), &expect![[r#" [ SrcLint { @@ -85,12 +93,6 @@ fn needless_parens_in_assignment() { message: "unnecessary parentheses", help: "remove the extra parentheses for clarity", }, - SrcLint { - source: "42", - level: Allow, - message: "this a placeholder", - help: "remove after addding the first HIR lint", - }, ] "#]], ); @@ -99,7 +101,7 @@ fn needless_parens_in_assignment() { #[test] fn needless_parens() { check( - "let x = (2) + (5 * 4 * (2 ^ 10));", + &wrap_in_callable("let x = (2) + (5 * 4 * (2 ^ 10));", CallableKind::Function), &expect![[r#" [ SrcLint { @@ -128,7 +130,7 @@ fn needless_parens() { #[test] fn redundant_semicolons() { check( - "let x = 2;;;;;", + &wrap_in_callable("let x = 2;;;;;", CallableKind::Function), &expect![[r#" [ SrcLint { @@ -143,16 +145,121 @@ fn redundant_semicolons() { } #[test] -fn hir_placeholder() { +fn needless_operation_lambda_operations() { check( - "let placeholder = 42;", + &wrap_in_callable("let a = (a) => a + 1;", CallableKind::Function), &expect![[r#" [ SrcLint { - source: "42", - level: Allow, - message: "this a placeholder", - help: "remove after addding the first HIR lint", + source: "", + level: Warn, + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", + }, + ] + "#]], + ); +} + +#[test] +fn needless_operation_no_lint_for_valid_lambda_operations() { + check( + &wrap_in_callable("let op = (q) => H(q);", CallableKind::Function), + &expect![[r" + [] + "]], + ); +} + +#[test] +fn needless_operation_non_empty_op_and_no_specialization() { + check( + &wrap_in_callable("let x = 2;", CallableKind::Operation), + &expect![[r#" + [ + SrcLint { + source: "RunProgram", + level: Warn, + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", + }, + ] + "#]], + ); +} + +#[test] +fn needless_operation_non_empty_op_and_specialization() { + check( + indoc! {" + operation Run(target : Qubit) : Unit is Adj { + body ... { + Message(\"hi\"); + } + adjoint self; + } + "}, + &expect![[r#" + [ + SrcLint { + source: "Run", + level: Warn, + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", + }, + ] + "#]], + ); +} + +#[test] +fn needless_operation_no_lint_for_empty_op_explicit_specialization() { + check( + indoc! {" + operation I(target : Qubit) : Unit { + body ... {} + adjoint self; + } + + "}, + &expect![[r" + [] + "]], + ); +} + +#[test] +fn needless_operation_no_lint_for_empty_op_implicit_specialization() { + check( + indoc! {" + operation DoNothing() : Unit is Adj + Ctl {} + "}, + &expect![[r" + [] + "]], + ); +} + +#[test] +fn needless_operation_partial_application() { + check( + indoc! {" + operation PrepareBellState(q1 : Qubit, q2 : Qubit) : Unit { + H(q1); + CNOT(q1, q2); + } + + operation PartialApplication(q1 : Qubit) : Qubit => Unit { + return PrepareBellState(q1, _); + } + "}, + &expect![[r#" + [ + SrcLint { + source: "PartialApplication", + level: Warn, + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", }, ] "#]], @@ -184,11 +291,17 @@ fn check(source: &str, expected: &Expect) { /// Wraps some source code into a namespace, to make testing easier. fn wrap_in_namespace(source: &str) -> String { format!( - "namespace foo {{ - operation RunProgram(vector : Double[]) : Unit {{ + "namespace Foo {{ + {source} + }}" + ) +} + +fn wrap_in_callable(source: &str, callable_type: CallableKind) -> String { + format!( + "{callable_type} RunProgram() : Unit {{ {source} - }} - }}" + }}" ) } diff --git a/language_service/src/code_action.rs b/language_service/src/code_action.rs index a1e00d23a8..2169fc592a 100644 --- a/language_service/src/code_action.rs +++ b/language_service/src/code_action.rs @@ -87,7 +87,8 @@ fn quick_fixes( kind: Some(CodeActionKind::QuickFix), is_preferred: None, }), - LintKind::Ast(AstLint::DivisionByZero) | LintKind::Hir(HirLint::Placeholder) => (), + LintKind::Ast(AstLint::DivisionByZero) + | LintKind::Hir(HirLint::NeedlessOperation) => (), } } } diff --git a/language_service/src/state/tests.rs b/language_service/src/state/tests.rs index 9264aa3e86..cf3836c410 100644 --- a/language_service/src/state/tests.rs +++ b/language_service/src/state/tests.rs @@ -685,8 +685,8 @@ fn notebook_document_lints() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "operation Foo() : Unit { let x = 4;;;; }"), - ("cell2", 1, "operation Bar() : Unit { let y = 5 / 0; }"), + ("cell1", 1, "function Foo() : Unit { let x = 4;;;; }"), + ("cell2", 1, "function Bar() : Unit { let y = 5 / 0; }"), ] .into_iter(), ); @@ -704,8 +704,8 @@ fn notebook_document_lints() { Lint( Lint { span: Span { - lo: 35, - hi: 38, + lo: 34, + hi: 37, }, level: Warn, message: "redundant semicolons", @@ -726,8 +726,8 @@ fn notebook_document_lints() { Lint( Lint { span: Span { - lo: 74, - hi: 79, + lo: 72, + hi: 77, }, level: Error, message: "attempt to divide by zero", @@ -1484,7 +1484,7 @@ async fn loading_lints_config_from_manifest() { #[tokio::test] async fn lints_update_after_manifest_change() { let this_file_qs = - "namespace Foo { @EntryPoint() operation Main() : Unit { let x = 5 / 0 + (2 ^ 4); } }"; + "namespace Foo { @EntryPoint() function Main() : Unit { let x = 5 / 0 + (2 ^ 4); } }"; let fs = FsNode::Dir( [dir( "project", @@ -1524,8 +1524,8 @@ async fn lints_update_after_manifest_change() { Lint( Lint { span: Span { - lo: 72, - hi: 79, + lo: 71, + hi: 78, }, level: Error, message: "unnecessary parentheses", @@ -1538,8 +1538,8 @@ async fn lints_update_after_manifest_change() { Lint( Lint { span: Span { - lo: 64, - hi: 69, + lo: 63, + hi: 68, }, level: Error, message: "attempt to divide by zero", @@ -1572,8 +1572,8 @@ async fn lints_update_after_manifest_change() { Lint( Lint { span: Span { - lo: 72, - hi: 79, + lo: 71, + hi: 78, }, level: Warn, message: "unnecessary parentheses", @@ -1586,8 +1586,8 @@ async fn lints_update_after_manifest_change() { Lint( Lint { span: Span { - lo: 64, - hi: 69, + lo: 63, + hi: 68, }, level: Warn, message: "attempt to divide by zero", diff --git a/samples/language/GettingStarted.qs b/samples/language/GettingStarted.qs index aed8b9ea91..7ede6f5a62 100644 --- a/samples/language/GettingStarted.qs +++ b/samples/language/GettingStarted.qs @@ -6,8 +6,7 @@ namespace MyQuantumProgram { @EntryPoint() - operation Main() : Result[] { + operation Main() : Unit { // TODO: Write your Q# code here. - return []; } }