[Unitary-Hack] Lint rule: Use functions

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

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, "unnecessary operation declaration", "convert to function")
+}
+
+/// 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<Lint>) {
-        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<Lint>) {
+        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.span));
+            }
         }
     }
 }

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: "operation RunProgram() : Unit {\n            let x = ((1 + 2)) / 0;;;;\n        }",
+                    level: Warn,
+                    message: "unnecessary operation declaration",
+                    help: "convert to function",
+                },
             ]
         "#]],
     );
@@ -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: "(a) => a + 1",
+                    level: Warn,
+                    message: "unnecessary operation declaration",
+                    help: "convert to function",
+                },
+            ]
+        "#]],
+    );
+}
+
+#[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: "operation RunProgram() : Unit {\n            let x = 2;\n        }",
+                    level: Warn,
+                    message: "unnecessary operation declaration",
+                    help: "convert to function",
+                },
+            ]
+        "#]],
+    );
+}
+
+#[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: "operation Run(target : Qubit) : Unit is Adj {\n    body ... {\n        Message(\"hi\");\n    }\n    adjoint self;\n}",
+                level: Warn,
+                message: "unnecessary operation declaration",
+                help: "convert to function",
+            },
+        ]
+    "#]],
+    );
+}
+
+#[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: "operation PartialApplication(q1 : Qubit) : Qubit => Unit {\n    return PrepareBellState(q1, _);\n}",
+                    level: Warn,
+                    message: "unnecessary operation declaration",
+                    help: "convert to function",
                 },
             ]
         "#]],
@@ -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}
-        }}
-    }}"
+        }}"
     )
 }
 

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) => (),
             }
         }
     }