[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,85 @@
-use qsc_hir::hir::Lit;
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+// use miette::Diagnostic;
+use qsc_data_structures::span::Span;
+use qsc_hir::{
+    hir::{CallableDecl, CallableKind, Expr, ExprKind, Stmt, StmtKind},
+    ty::Ty,
+    visit::{self, Visitor},
+};
+// use thiserror::Error;
 
 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")
+}
+
+#[derive(Default)]
+pub(super) struct OperationLimits {
+    // Operation Characteristics
+    pub(super) op_char: Vec<Span>,
+}
+
+impl Visitor<'_> for OperationLimits {
+    fn visit_callable_decl(&mut self, decl: &CallableDecl) {
+        if decl.kind == CallableKind::Operation {
+            // Checking for special Ctl and Adj implementations
+            // if decl.adj.is_some() || decl.ctl.is_some() || decl.ctl_adj.is_some() {
+            //     self.op_char.push(decl.span);
+            // }
+            // if decl.functors != FunctorSetValue::Empty {
+            //     self.op_char.push(decl.name.span);
+            // }
+            visit::walk_callable_decl(self, decl);
+        }
+    }
+
+    fn visit_stmt(&mut self, stmt: &Stmt) {
+        if self.op_char.is_empty() {
+            if let StmtKind::Qubit(..) = &stmt.kind {
+                self.op_char.push(stmt.span);
+            } else {
+                visit::walk_stmt(self, stmt);
+            }
+        }
+    }
+
+    fn visit_expr(&mut self, expr: &Expr) {
+        if self.op_char.is_empty() {
+            match &expr.kind {
+                ExprKind::Call(callee, _) => {
+                    if matches!(&callee.ty, Ty::Arrow(arrow) if arrow.kind == CallableKind::Operation)
+                    {
+                        self.op_char.push(expr.span);
+                    }
+                }
+                ExprKind::Conjugate(..) | ExprKind::Repeat(..) => {
+                    self.op_char.push(expr.span);
+                }
+                _ => {
+                    visit::walk_expr(self, expr);
+                }
+            }
+        }
+    }
 }
 
-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 HirLintPass for NeedlessOperation {
+    fn check_callable_decl(&self, decl: &CallableDecl, buffer: &mut Vec<Lint>) {
+        if decl.kind == CallableKind::Operation {
+            let mut op_limits = OperationLimits::default();
+
+            op_limits.visit_callable_decl(decl);
+
+            let op_char = op_limits.op_char;
+
+            if op_char.is_empty() {
+                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,42 @@ fn redundant_semicolons() {
 }
 
 #[test]
-fn hir_placeholder() {
+fn needless_operation() {
     check(
-        "let placeholder = 42;",
+        &wrap_in_callable("let x = 2;", CallableKind::Operation),
         &expect![[r#"
             [
                 SrcLint {
-                    source: "42",
-                    level: Allow,
-                    message: "this a placeholder",
-                    help: "remove after addding the first HIR lint",
+                    source: "operation RunProgram() : Unit {\n            let x = 2;\n        }",
+                    level: Warn,
+                    message: "unnecessary operation declaration",
+                    help: "convert to function",
+                },
+            ]
+        "#]],
+    );
+}
+
+#[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",
                 },
             ]
         "#]],
@@ -161,6 +189,7 @@ fn hir_placeholder() {
 
 fn check(source: &str, expected: &Expect) {
     let source = wrap_in_namespace(source);
+
     let mut store = PackageStore::new(compile::core());
     let std = store.insert(compile::std(&store, TargetCapabilityFlags::all()));
     let sources = SourceMap::new([("source.qs".into(), source.clone().into())], None);
@@ -184,11 +213,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}
-        }}
-    }}"
+        }}"
     )
 }