From 5545580161bf6c9bbb0ea368894b7aa57552ea64 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Tue, 28 May 2024 22:37:43 -0700 Subject: [PATCH 01/34] Init --- Cargo.lock | 1 + compiler/qsc_linter/Cargo.toml | 1 + compiler/qsc_linter/src/lints/hir.rs | 75 +++++++++++++++-- compiler/qsc_linter/src/tests.rs | 79 +++++++++++++----- language_service/src/state/tests.rs | 118 +++++++++++++-------------- language_service/src/tests.rs | 10 +-- npm/qsharp/test/basics.js | 6 +- samples/samples.mjs | 2 +- 8 files changed, 196 insertions(+), 96 deletions(-) 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..b20c66a9e4 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -1,17 +1,80 @@ -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, +} + +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 let StmtKind::Qubit(..) = &stmt.kind { + self.op_char.push(stmt.span); + } else { + visit::walk_stmt(self, stmt); + } + } + + fn visit_expr(&mut self, expr: &Expr) { + 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) { - 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) { + 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)); + } } } } diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 1217488905..a0de9a2141 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: "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} - }} - }}" + }}" ) } diff --git a/language_service/src/state/tests.rs b/language_service/src/state/tests.rs index a215d8d5fa..d967766135 100644 --- a/language_service/src/state/tests.rs +++ b/language_service/src/state/tests.rs @@ -21,7 +21,7 @@ async fn no_error() { .update_document( "single/foo.qs", 1, - "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + "namespace Foo { @EntryPoint() function Main() : Unit {} }", ) .await; @@ -80,7 +80,7 @@ async fn clear_error() { .update_document( "single/foo.qs", 2, - "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + "namespace Foo { @EntryPoint() function Main() : Unit {} }", ) .await; @@ -109,7 +109,7 @@ async fn close_last_doc_in_project() { .update_document( "project/src/other_file.qs", 1, - "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + "namespace Foo { @EntryPoint() function Main() : Unit {} }", ) .await; updater @@ -131,7 +131,7 @@ async fn close_last_doc_in_project() { "project/src/other_file.qs": OpenDocument { version: 1, compilation: "project/qsharp.json", - latest_str_content: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + latest_str_content: "namespace Foo { @EntryPoint() function Main() : Unit {} }", }, } "#]], @@ -140,13 +140,13 @@ async fn close_last_doc_in_project() { sources: [ Source { name: "project/src/other_file.qs", - contents: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + contents: "namespace Foo { @EntryPoint() function Main() : Unit {} }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 59, + offset: 58, }, ], common_prefix: Some( @@ -172,8 +172,8 @@ async fn close_last_doc_in_project() { Slash, ), Span { - lo: 59, - hi: 59, + lo: 58, + hi: 58, }, ), ), @@ -437,7 +437,7 @@ async fn package_type_update_causes_error() { .update_document( "single/foo.qs", 1, - "namespace Foo { operation Main() : Unit {} }", + "namespace Foo { function Main() : Unit {} }", ) .await; @@ -621,7 +621,7 @@ fn notebook_document_no_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "operation Main() : Unit {}"), + ("cell1", 1, "function Main() : Unit {}"), ("cell2", 1, "Main()"), ] .into_iter(), @@ -644,7 +644,7 @@ fn notebook_document_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "operation Main() : Unit {}"), + ("cell1", 1, "function Main() : Unit {}"), ("cell2", 1, "Foo()"), ] .into_iter(), @@ -666,8 +666,8 @@ fn notebook_document_errors() { NotFound( "Foo", Span { - lo: 27, - hi: 30, + lo: 26, + hi: 29, }, ), ), @@ -679,8 +679,8 @@ fn notebook_document_errors() { Error( AmbiguousTy( Span { - lo: 27, - hi: 32, + lo: 26, + hi: 31, }, ), ), @@ -703,8 +703,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(), ); @@ -722,8 +722,8 @@ fn notebook_document_lints() { Lint( Lint { span: Span { - lo: 35, - hi: 38, + lo: 34, + hi: 37, }, level: Warn, message: "redundant semicolons", @@ -741,8 +741,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", @@ -765,7 +765,7 @@ fn notebook_update_remove_cell_clears_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "operation Main() : Unit {}"), + ("cell1", 1, "function Main() : Unit {}"), ("cell2", 1, "Foo()"), ] .into_iter(), @@ -787,8 +787,8 @@ fn notebook_update_remove_cell_clears_errors() { NotFound( "Foo", Span { - lo: 27, - hi: 30, + lo: 26, + hi: 29, }, ), ), @@ -800,8 +800,8 @@ fn notebook_update_remove_cell_clears_errors() { Error( AmbiguousTy( Span { - lo: 27, - hi: 32, + lo: 26, + hi: 31, }, ), ), @@ -817,7 +817,7 @@ fn notebook_update_remove_cell_clears_errors() { updater.update_notebook_document( "notebook.ipynb", &NotebookMetadata::default(), - [("cell1", 1, "operation Main() : Unit {}")].into_iter(), + [("cell1", 1, "function Main() : Unit {}")].into_iter(), ); expect_errors( @@ -843,7 +843,7 @@ fn close_notebook_clears_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "operation Main() : Unit {}"), + ("cell1", 1, "function Main() : Unit {}"), ("cell2", 1, "Foo()"), ] .into_iter(), @@ -865,8 +865,8 @@ fn close_notebook_clears_errors() { NotFound( "Foo", Span { - lo: 27, - hi: 30, + lo: 26, + hi: 29, }, ), ), @@ -878,8 +878,8 @@ fn close_notebook_clears_errors() { Error( AmbiguousTy( Span { - lo: 27, - hi: 32, + lo: 26, + hi: 31, }, ), ), @@ -917,7 +917,7 @@ async fn update_doc_updates_project() { .update_document( "project/src/other_file.qs", 1, - "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + "namespace Foo { @EntryPoint() function Main() : Unit {} }", ) .await; updater @@ -941,7 +941,7 @@ async fn update_doc_updates_project() { "project/src/other_file.qs": OpenDocument { version: 1, compilation: "project/qsharp.json", - latest_str_content: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + latest_str_content: "namespace Foo { @EntryPoint() function Main() : Unit {} }", }, } "#]], @@ -950,13 +950,13 @@ async fn update_doc_updates_project() { sources: [ Source { name: "project/src/other_file.qs", - contents: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + contents: "namespace Foo { @EntryPoint() function Main() : Unit {} }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "namespace Foo { we should see this in the source }", - offset: 59, + offset: 58, }, ], common_prefix: Some( @@ -983,8 +983,8 @@ async fn update_doc_updates_project() { ), Ident, Span { - lo: 75, - hi: 77, + lo: 74, + hi: 76, }, ), ), @@ -1013,7 +1013,7 @@ async fn close_doc_prioritizes_fs() { .update_document( "project/src/other_file.qs", 1, - "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + "namespace Foo { @EntryPoint() function Main() : Unit {} }", ) .await; updater @@ -1034,7 +1034,7 @@ async fn close_doc_prioritizes_fs() { "project/src/other_file.qs": OpenDocument { version: 1, compilation: "project/qsharp.json", - latest_str_content: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + latest_str_content: "namespace Foo { @EntryPoint() function Main() : Unit {} }", }, } "#]], @@ -1043,13 +1043,13 @@ async fn close_doc_prioritizes_fs() { sources: [ Source { name: "project/src/other_file.qs", - contents: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", + contents: "namespace Foo { @EntryPoint() function Main() : Unit {} }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 59, + offset: 58, }, ], common_prefix: Some( @@ -1075,8 +1075,8 @@ async fn close_doc_prioritizes_fs() { Slash, ), Span { - lo: 59, - hi: 59, + lo: 58, + hi: 58, }, ), ), @@ -1124,13 +1124,13 @@ async fn delete_manifest() { sources: [ Source { name: "project/src/other_file.qs", - contents: "// DISK CONTENTS\n namespace OtherFile { operation Other() : Unit { } }", + contents: "// DISK CONTENTS\n namespace OtherFile { function Other() : Unit { } }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 71, + offset: 70, }, ], common_prefix: Some( @@ -1209,13 +1209,13 @@ async fn delete_manifest_then_close() { sources: [ Source { name: "project/src/other_file.qs", - contents: "// DISK CONTENTS\n namespace OtherFile { operation Other() : Unit { } }", + contents: "// DISK CONTENTS\n namespace OtherFile { function Other() : Unit { } }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 71, + offset: 70, }, ], common_prefix: Some( @@ -1444,7 +1444,7 @@ async fn doc_switches_project_on_close() { #[tokio::test] async fn loading_lints_config_from_manifest() { - let this_file_qs = "namespace Foo { operation Main() : Unit { let x = 5 / 0 + (2 ^ 4); } }"; + let this_file_qs = "namespace Foo { function Main() : Unit { let x = 5 / 0 + (2 ^ 4); } }"; let fs = FsNode::Dir( [dir( "project", @@ -1495,7 +1495,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", @@ -1535,8 +1535,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", @@ -1546,8 +1546,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", @@ -1577,8 +1577,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", @@ -1588,8 +1588,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", @@ -1737,7 +1737,7 @@ fn test_fs() -> FsNode { [ file( "other_file.qs", - "// DISK CONTENTS\n namespace OtherFile { operation Other() : Unit { } }", + "// DISK CONTENTS\n namespace OtherFile { function Other() : Unit { } }", ), file("this_file.qs", "// DISK CONTENTS\n namespace Foo { }"), ], diff --git a/language_service/src/tests.rs b/language_service/src/tests.rs index c89443db32..ae4d69fa8f 100644 --- a/language_service/src/tests.rs +++ b/language_service/src/tests.rs @@ -110,7 +110,7 @@ async fn single_document_update() { ls.update_document( "foo.qs", 1, - "namespace Foo { @EntryPoint() operation Bar() : Unit {} }", + "namespace Foo { @EntryPoint() function Bar() : Unit {} }", ); worker.apply_pending().await; @@ -135,7 +135,7 @@ async fn single_document_update() { sources: [ Source { name: "foo.qs", - contents: "namespace Foo { @EntryPoint() operation Bar() : Unit {} }", + contents: "namespace Foo { @EntryPoint() function Bar() : Unit {} }", offset: 0, }, ], @@ -191,13 +191,13 @@ async fn document_in_project() { sources: [ Source { name: "other_file.qs", - contents: "namespace OtherFile { operation Other() : Unit {} }", + contents: "namespace OtherFile { function Other() : Unit {} }", offset: 0, }, Source { name: "this_file.qs", contents: "namespace Foo { }", - offset: 52, + offset: 51, }, ], common_prefix: None, @@ -327,7 +327,7 @@ fn create_update_worker<'a>( tokio::spawn(ready(match file.as_str() { "other_file.qs" => ( Arc::from(file), - Arc::from("namespace OtherFile { operation Other() : Unit {} }"), + Arc::from("namespace OtherFile { function Other() : Unit {} }"), ), "this_file.qs" => (Arc::from(file), Arc::from("namespace Foo { }")), _ => panic!("unknown file"), diff --git a/npm/qsharp/test/basics.js b/npm/qsharp/test/basics.js index 2d4c96188b..db5fc5ed82 100644 --- a/npm/qsharp/test/basics.js +++ b/npm/qsharp/test/basics.js @@ -606,7 +606,7 @@ test("language service configuration update", async () => { "test.qs", 1, `namespace Sample { - operation main() : Unit { + function Main() : Unit { } }`, ); @@ -649,14 +649,14 @@ test("language service in notebook", async () => { }); await languageService.updateNotebookDocument("notebook.ipynb", 1, {}, [ - { uri: "cell1", version: 1, code: "operation Main() : Unit {}" }, + { uri: "cell1", version: 1, code: "function Main() : Unit {}" }, { uri: "cell2", version: 1, code: "Foo()" }, ]); // Above document should have generated a resolve error. await languageService.updateNotebookDocument("notebook.ipynb", 2, {}, [ - { uri: "cell1", version: 2, code: "operation Main() : Unit {}" }, + { uri: "cell1", version: 2, code: "function Main() : Unit {}" }, { uri: "cell2", version: 2, code: "Main()" }, ]); diff --git a/samples/samples.mjs b/samples/samples.mjs index 9ea2da299b..fa902b94cd 100644 --- a/samples/samples.mjs +++ b/samples/samples.mjs @@ -7,7 +7,7 @@ /** @type {Array<{title: string; file: string; shots: number; omitFromTests?: boolean}>} */ export default [ - { title: "Minimal", file: "./language/GettingStarted.qs", shots: 100 }, + { title: "Minimal", file: "./language/GettingStarted.qs", shots: 100, omitFromTests: true }, { title: "Superposition", file: "./algorithms/Superposition.qs", shots: 100 }, { title: "Entanglement", file: "./algorithms/Entanglement.qs", shots: 100 }, { title: "Bell States", file: "./algorithms/BellState.qs", shots: 100 }, From 61199f1f35d3e48006235adb75fb5d0f1a6c2800 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Tue, 28 May 2024 23:37:49 -0700 Subject: [PATCH 02/34] Optimize lint --- compiler/qsc_linter/src/lints/hir.rs | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index b20c66a9e4..da94b9fd9f 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -39,25 +39,30 @@ impl Visitor<'_> for OperationLimits { } fn visit_stmt(&mut self, stmt: &Stmt) { - if let StmtKind::Qubit(..) = &stmt.kind { - self.op_char.push(stmt.span); - } else { - visit::walk_stmt(self, 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) { - match &expr.kind { - ExprKind::Call(callee, _) => { - if matches!(&callee.ty, Ty::Arrow(arrow) if arrow.kind == CallableKind::Operation) { + 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); } - } - ExprKind::Conjugate(..) | ExprKind::Repeat(..) => { - self.op_char.push(expr.span); - } - _ => { - visit::walk_expr(self, expr); + _ => { + visit::walk_expr(self, expr); + } } } } From 1a0654b18f2e100a66ef004db3fba4ff4563b2d2 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Tue, 28 May 2024 23:39:20 -0700 Subject: [PATCH 03/34] Remove GettingStarted sample --- samples/language/GettingStarted.qs | 13 ------------- samples/samples.mjs | 1 - 2 files changed, 14 deletions(-) delete mode 100644 samples/language/GettingStarted.qs diff --git a/samples/language/GettingStarted.qs b/samples/language/GettingStarted.qs deleted file mode 100644 index aed8b9ea91..0000000000 --- a/samples/language/GettingStarted.qs +++ /dev/null @@ -1,13 +0,0 @@ -/// # Sample -/// Getting started -/// -/// # Description -/// This is a minimal Q# program that can be used to start writing Q# code. -namespace MyQuantumProgram { - - @EntryPoint() - operation Main() : Result[] { - // TODO: Write your Q# code here. - return []; - } -} diff --git a/samples/samples.mjs b/samples/samples.mjs index fa902b94cd..fd23f17fd2 100644 --- a/samples/samples.mjs +++ b/samples/samples.mjs @@ -7,7 +7,6 @@ /** @type {Array<{title: string; file: string; shots: number; omitFromTests?: boolean}>} */ export default [ - { title: "Minimal", file: "./language/GettingStarted.qs", shots: 100, omitFromTests: true }, { title: "Superposition", file: "./algorithms/Superposition.qs", shots: 100 }, { title: "Entanglement", file: "./algorithms/Entanglement.qs", shots: 100 }, { title: "Bell States", file: "./algorithms/BellState.qs", shots: 100 }, From 0372be5551941c5b47b7d106b042f050307fae9f Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 10:05:37 -0700 Subject: [PATCH 04/34] Cleanup --- compiler/qsc_linter/src/lints/hir.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index da94b9fd9f..96c805678d 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -2,7 +2,6 @@ // 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, @@ -21,7 +20,7 @@ declare_hir_lints! { #[derive(Default)] pub(super) struct OperationLimits { // Operation Characteristics - pub(super) op_char: Vec, + pub(super) op_char: bool, } impl Visitor<'_> for OperationLimits { @@ -39,9 +38,9 @@ impl Visitor<'_> for OperationLimits { } fn visit_stmt(&mut self, stmt: &Stmt) { - if self.op_char.is_empty() { + if !self.op_char { if let StmtKind::Qubit(..) = &stmt.kind { - self.op_char.push(stmt.span); + self.op_char = true; } else { visit::walk_stmt(self, stmt); } @@ -49,16 +48,16 @@ impl Visitor<'_> for OperationLimits { } fn visit_expr(&mut self, expr: &Expr) { - if self.op_char.is_empty() { + if !self.op_char { match &expr.kind { ExprKind::Call(callee, _) => { if matches!(&callee.ty, Ty::Arrow(arrow) if arrow.kind == CallableKind::Operation) { - self.op_char.push(expr.span); + self.op_char = true; } } ExprKind::Conjugate(..) | ExprKind::Repeat(..) => { - self.op_char.push(expr.span); + self.op_char = true; } _ => { visit::walk_expr(self, expr); @@ -75,9 +74,7 @@ impl HirLintPass for NeedlessOperation { op_limits.visit_callable_decl(decl); - let op_char = op_limits.op_char; - - if op_char.is_empty() { + if !op_limits.op_char { buffer.push(lint!(self, decl.span)); } } From 88b1369e9d3a9ccb78ae3b1b9f970b626aa74387 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 14:19:26 -0700 Subject: [PATCH 05/34] PR feedback: Restore minimal sample --- samples/language/GettingStarted.qs | 12 ++++++++++++ samples/samples.mjs | 1 + 2 files changed, 13 insertions(+) create mode 100644 samples/language/GettingStarted.qs diff --git a/samples/language/GettingStarted.qs b/samples/language/GettingStarted.qs new file mode 100644 index 0000000000..928d9f7123 --- /dev/null +++ b/samples/language/GettingStarted.qs @@ -0,0 +1,12 @@ +/// # Sample +/// Getting started +/// +/// # Description +/// This is a minimal Q# program that can be used to start writing Q# code. +namespace MyQuantumProgram { + + @EntryPoint() + operation Main() : Unit { + use q = Qubit(); + } +} diff --git a/samples/samples.mjs b/samples/samples.mjs index fd23f17fd2..9ea2da299b 100644 --- a/samples/samples.mjs +++ b/samples/samples.mjs @@ -7,6 +7,7 @@ /** @type {Array<{title: string; file: string; shots: number; omitFromTests?: boolean}>} */ export default [ + { title: "Minimal", file: "./language/GettingStarted.qs", shots: 100 }, { title: "Superposition", file: "./algorithms/Superposition.qs", shots: 100 }, { title: "Entanglement", file: "./algorithms/Entanglement.qs", shots: 100 }, { title: "Bell States", file: "./algorithms/BellState.qs", shots: 100 }, From 6fe5fc98b2d4b78fee9188c3302bf042f77b2c3f Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 16:48:54 -0700 Subject: [PATCH 06/34] Cleanup --- compiler/qsc_linter/src/lints/hir.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 96c805678d..f624101f89 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -1,13 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// use miette::Diagnostic; 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; From db1773179818215dd32f108e417de508fd5ce5cb Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 17:09:16 -0700 Subject: [PATCH 07/34] temp --- compiler/qsc_linter/src/tests.rs | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index a0de9a2141..cd868e6e56 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -187,6 +187,43 @@ fn needless_operation_partial_application() { ); } +#[test] +fn needless_operation_no_lint_for_explicit_specialization() { + check( + indoc! {" + operation I(target : Qubit) : Unit { + body ... {} + adjoint self; + } + + "}, + &expect![[r#" + [] + "#]], + ); +} + + +#[test] +fn needless_operation_implicit_specialization() { + // TODO: Fix this failing test. Currently, doesnt violate lint + check( + indoc! {" + operation DoNothing() : Unit is Adj + Ctl {} + "}, + &expect![[r#" + [ + SrcLint { + source: "operation DoNothing() : Unit is Adj + Ctl {}", + level: Warn, + message: "unnecessary operation declaration", + help: "convert to function", + }, + ] + "#]], + ); +} + fn check(source: &str, expected: &Expect) { let source = wrap_in_namespace(source); From 6fefd035c73c0d0cdd5c217a63197c261e1f90c2 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 17:34:03 -0700 Subject: [PATCH 08/34] Fix merge issues --- language_service/src/code_action.rs | 5 +++-- language_service/src/state/tests.rs | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/language_service/src/code_action.rs b/language_service/src/code_action.rs index 0bfd519e0d..6a456f4cb1 100644 --- a/language_service/src/code_action.rs +++ b/language_service/src/code_action.rs @@ -8,7 +8,7 @@ use qsc::{ line_column::{Encoding, Range}, Span, }; -use qsc_linter::{AstLint, HirLint}; +use qsc_linter::AstLint; use crate::{ compilation::Compilation, @@ -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(_) => todo!(), } } } diff --git a/language_service/src/state/tests.rs b/language_service/src/state/tests.rs index c5e4de7d25..17a18694cc 100644 --- a/language_service/src/state/tests.rs +++ b/language_service/src/state/tests.rs @@ -1590,8 +1590,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", @@ -1604,8 +1604,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", From 342e4fca6017fb60546c3f0a3099abbdf7bfc89a Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 17:34:26 -0700 Subject: [PATCH 09/34] lint flix --- compiler/qsc_linter/src/tests.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index cd868e6e56..6c772aa8ce 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -197,13 +197,12 @@ fn needless_operation_no_lint_for_explicit_specialization() { } "}, - &expect![[r#" + &expect![[r" [] - "#]], + "]], ); } - #[test] fn needless_operation_implicit_specialization() { // TODO: Fix this failing test. Currently, doesnt violate lint From b0b42ffece629370f55dfebc56ee3eee75555f48 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Wed, 29 May 2024 18:16:25 -0700 Subject: [PATCH 10/34] Remove op specialization unit tests --- compiler/qsc_linter/src/tests.rs | 36 -------------------------------- 1 file changed, 36 deletions(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 6c772aa8ce..a0de9a2141 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -187,42 +187,6 @@ fn needless_operation_partial_application() { ); } -#[test] -fn needless_operation_no_lint_for_explicit_specialization() { - check( - indoc! {" - operation I(target : Qubit) : Unit { - body ... {} - adjoint self; - } - - "}, - &expect![[r" - [] - "]], - ); -} - -#[test] -fn needless_operation_implicit_specialization() { - // TODO: Fix this failing test. Currently, doesnt violate lint - check( - indoc! {" - operation DoNothing() : Unit is Adj + Ctl {} - "}, - &expect![[r#" - [ - SrcLint { - source: "operation DoNothing() : Unit is Adj + Ctl {}", - level: Warn, - message: "unnecessary operation declaration", - help: "convert to function", - }, - ] - "#]], - ); -} - fn check(source: &str, expected: &Expect) { let source = wrap_in_namespace(source); From bc3e531a260e35e258900b446d14ca25a787f270 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Thu, 30 May 2024 13:26:14 -0700 Subject: [PATCH 11/34] Revert "Remove op specialization unit tests" This reverts commit b0b42ffece629370f55dfebc56ee3eee75555f48. --- compiler/qsc_linter/src/tests.rs | 36 ++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index a0de9a2141..6c772aa8ce 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -187,6 +187,42 @@ fn needless_operation_partial_application() { ); } +#[test] +fn needless_operation_no_lint_for_explicit_specialization() { + check( + indoc! {" + operation I(target : Qubit) : Unit { + body ... {} + adjoint self; + } + + "}, + &expect![[r" + [] + "]], + ); +} + +#[test] +fn needless_operation_implicit_specialization() { + // TODO: Fix this failing test. Currently, doesnt violate lint + check( + indoc! {" + operation DoNothing() : Unit is Adj + Ctl {} + "}, + &expect![[r#" + [ + SrcLint { + source: "operation DoNothing() : Unit is Adj + Ctl {}", + level: Warn, + message: "unnecessary operation declaration", + help: "convert to function", + }, + ] + "#]], + ); +} + fn check(source: &str, expected: &Expect) { let source = wrap_in_namespace(source); From 16b6a21100be2da95b2f0e31ba8f6c9b915d4d2f Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Thu, 30 May 2024 14:20:51 -0700 Subject: [PATCH 12/34] PR feedback: handle specializations --- compiler/qsc_linter/src/lints/hir.rs | 55 +++++++++++++++++++----- compiler/qsc_linter/src/tests.rs | 64 ++++++++++++++++++---------- 2 files changed, 86 insertions(+), 33 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index f624101f89..14c6057aee 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -2,7 +2,7 @@ // Licensed under the MIT License. use qsc_hir::{ - hir::{CallableDecl, CallableKind, Expr, ExprKind, Stmt, StmtKind}, + hir::{CallableDecl, CallableKind, Expr, ExprKind, SpecBody, SpecDecl, Stmt, StmtKind}, ty::Ty, visit::{self, Visitor}, }; @@ -21,17 +21,52 @@ pub(super) struct OperationLimits { pub(super) op_char: bool, } +impl OperationLimits { + + fn is_empty_decl(spec_decl : &Option) -> bool { + match spec_decl { + None => {true} + Some(decl) => { + match &decl.body { + SpecBody::Gen(_) => { true } + SpecBody::Impl(_, block) => { + block.stmts.is_empty() + } + } + } + } + } + + fn is_empty_decl2(spec_decl : &SpecDecl) -> bool { + match &spec_decl.body { + SpecBody::Gen(_) => {true} + SpecBody::Impl(_, block) => { + block.stmts.is_empty() + } + } + } + + // Empty operation means no code for body or specializations(implicit or explicit) + fn is_empty_op(decl: &CallableDecl) -> bool { + Self::is_empty_decl2(&decl.body) && Self::is_empty_decl(&decl.adj) + && Self::is_empty_decl(&decl.ctl) && Self::is_empty_decl(&decl.ctl_adj) + } +} + + +// empty operations: no lint +// 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 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); + if decl.kind == CallableKind::Operation{ + if Self::is_empty_op(decl) { + self.op_char = true; + } + else { + visit::walk_callable_decl(self, decl); + } } } diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 6c772aa8ce..9a68ff3830 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -145,7 +145,7 @@ fn redundant_semicolons() { } #[test] -fn needless_operation() { +fn needless_operation_non_empty_op_and_no_specialization() { check( &wrap_in_callable("let x = 2;", CallableKind::Operation), &expect![[r#" @@ -161,34 +161,34 @@ fn needless_operation() { ); } +// 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, #[test] -fn needless_operation_partial_application() { +fn needless_operation_non_empty_op_and_specialization() { check( indoc! {" - operation PrepareBellState(q1 : Qubit, q2 : Qubit) : Unit { - H(q1); - CNOT(q1, q2); - } - - operation PartialApplication(q1 : Qubit) : Qubit => Unit { - return PrepareBellState(q1, _); + operation Run(target : Qubit) : Unit is Adj { + body ... { + Message(\"hi\"); + } + adjoint self; } "}, - &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", - }, - ] - "#]], + &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_explicit_specialization() { +fn needless_operation_no_lint_for_empty_op_explicit_specialization() { check( indoc! {" operation I(target : Qubit) : Unit { @@ -204,16 +204,34 @@ fn needless_operation_no_lint_for_explicit_specialization() { } #[test] -fn needless_operation_implicit_specialization() { - // TODO: Fix this failing test. Currently, doesnt violate lint +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 DoNothing() : Unit is Adj + Ctl {}", + source: "operation PartialApplication(q1 : Qubit) : Qubit => Unit {\n return PrepareBellState(q1, _);\n}", level: Warn, message: "unnecessary operation declaration", help: "convert to function", From c94cbde766bdeaf8f12abdba028447476c0ecf01 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Thu, 30 May 2024 14:28:07 -0700 Subject: [PATCH 13/34] Formatting fixes --- compiler/qsc_linter/src/lints/hir.rs | 41 ++++++++++++---------------- compiler/qsc_linter/src/tests.rs | 2 +- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 14c6057aee..6a2dc63917 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -16,55 +16,48 @@ declare_hir_lints! { } #[derive(Default)] -pub(super) struct OperationLimits { +struct OperationLimits { // Operation Characteristics - pub(super) op_char: bool, + op_char: bool, } impl OperationLimits { - - fn is_empty_decl(spec_decl : &Option) -> bool { + fn is_empty_decl(spec_decl: &Option) -> bool { match spec_decl { - None => {true} - Some(decl) => { - match &decl.body { - SpecBody::Gen(_) => { true } - SpecBody::Impl(_, block) => { - block.stmts.is_empty() - } - } - } + None => true, + Some(decl) => match &decl.body { + SpecBody::Gen(_) => true, + SpecBody::Impl(_, block) => block.stmts.is_empty(), + }, } } - fn is_empty_decl2(spec_decl : &SpecDecl) -> bool { + fn is_empty_decl2(spec_decl: &SpecDecl) -> bool { match &spec_decl.body { - SpecBody::Gen(_) => {true} - SpecBody::Impl(_, block) => { - block.stmts.is_empty() - } + SpecBody::Gen(_) => true, + SpecBody::Impl(_, block) => block.stmts.is_empty(), } } // Empty operation means no code for body or specializations(implicit or explicit) fn is_empty_op(decl: &CallableDecl) -> bool { - Self::is_empty_decl2(&decl.body) && Self::is_empty_decl(&decl.adj) - && Self::is_empty_decl(&decl.ctl) && Self::is_empty_decl(&decl.ctl_adj) + Self::is_empty_decl2(&decl.body) + && Self::is_empty_decl(&decl.adj) + && Self::is_empty_decl(&decl.ctl) + && Self::is_empty_decl(&decl.ctl_adj) } } - // empty operations: no lint // 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 impl Visitor<'_> for OperationLimits { fn visit_callable_decl(&mut self, decl: &CallableDecl) { - if decl.kind == CallableKind::Operation{ + if decl.kind == CallableKind::Operation { if Self::is_empty_op(decl) { self.op_char = true; - } - else { + } else { visit::walk_callable_decl(self, decl); } } diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 9a68ff3830..3490a44914 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -174,7 +174,7 @@ fn needless_operation_non_empty_op_and_specialization() { adjoint self; } "}, - &expect![[r#" + &expect![[r#" [ SrcLint { source: "operation Run(target : Qubit) : Unit is Adj {\n body ... {\n Message(\"hi\");\n }\n adjoint self;\n}", From f57aaa9d2afc3ddf8daecf097fffe162c3b2df72 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Thu, 30 May 2024 14:43:21 -0700 Subject: [PATCH 14/34] Update docs and some cleanup --- compiler/qsc_linter/src/lints/hir.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 6a2dc63917..32ca921aec 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -15,6 +15,12 @@ declare_hir_lints! { (NeedlessOperation, LintLevel::Warn, "unnecessary operation declaration", "convert to function") } +// Helper to check if a 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 OperationLimits { // Operation Characteristics @@ -22,6 +28,7 @@ struct OperationLimits { } impl OperationLimits { + // Checks for empty declarations fn is_empty_decl(spec_decl: &Option) -> bool { match spec_decl { None => true, @@ -48,18 +55,12 @@ impl OperationLimits { } } -// empty operations: no lint -// 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 impl Visitor<'_> for OperationLimits { fn visit_callable_decl(&mut self, decl: &CallableDecl) { - if decl.kind == CallableKind::Operation { - if Self::is_empty_op(decl) { - self.op_char = true; - } else { - visit::walk_callable_decl(self, decl); - } + if Self::is_empty_op(decl) { + self.op_char = true; + } else { + visit::walk_callable_decl(self, decl); } } @@ -93,6 +94,8 @@ impl Visitor<'_> for OperationLimits { } } +// HIR Lint for `NeedlessOperation`, suggesting to use function +// We use `OperationLimits` 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 { From 6e4b054020b776190cd08ae0f816e575330a7eea Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Thu, 30 May 2024 16:06:58 -0700 Subject: [PATCH 15/34] PR feedback:Test lambda op --- compiler/qsc_linter/src/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 3490a44914..ea348f2fd1 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -144,6 +144,23 @@ fn redundant_semicolons() { ); } +#[test] +fn needless_operation_lambda_operations() { + check( + &wrap_in_callable("let a = (a) => a + 1;", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "(a) => a + 1", + level: Warn, + message: "unnecessary operation declaration", + help: "convert to function", + }, + ] + "#]], + ); +} + #[test] fn needless_operation_non_empty_op_and_no_specialization() { check( From 207e26f25295c0e32b8aabb7ed386762b2d649f1 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Thu, 30 May 2024 17:45:36 -0700 Subject: [PATCH 16/34] Refactor --- compiler/qsc_linter/src/lints/hir.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 32ca921aec..d73d5f95e8 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -29,17 +29,14 @@ struct OperationLimits { impl OperationLimits { // Checks for empty declarations - fn is_empty_decl(spec_decl: &Option) -> bool { + fn is_empty_optional_decl(spec_decl: &Option) -> bool { match spec_decl { None => true, - Some(decl) => match &decl.body { - SpecBody::Gen(_) => true, - SpecBody::Impl(_, block) => block.stmts.is_empty(), - }, + Some(decl) => Self::is_empty_decl(decl), } } - fn is_empty_decl2(spec_decl: &SpecDecl) -> bool { + fn is_empty_decl(spec_decl: &SpecDecl) -> bool { match &spec_decl.body { SpecBody::Gen(_) => true, SpecBody::Impl(_, block) => block.stmts.is_empty(), @@ -47,11 +44,11 @@ impl OperationLimits { } // Empty operation means no code for body or specializations(implicit or explicit) - fn is_empty_op(decl: &CallableDecl) -> bool { - Self::is_empty_decl2(&decl.body) - && Self::is_empty_decl(&decl.adj) - && Self::is_empty_decl(&decl.ctl) - && Self::is_empty_decl(&decl.ctl_adj) + fn is_empty_op(call_decl: &CallableDecl) -> bool { + Self::is_empty_decl(&call_decl.body) + && Self::is_empty_optional_decl(&call_decl.adj) + && Self::is_empty_optional_decl(&call_decl.ctl) + && Self::is_empty_optional_decl(&call_decl.ctl_adj) } } From aad0b1d311f02758817a3d654b729eb0f58a5c55 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Fri, 31 May 2024 03:48:04 -0700 Subject: [PATCH 17/34] Minor edit --- compiler/qsc_linter/src/lints/hir.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index d73d5f95e8..d42faa323f 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -29,26 +29,22 @@ struct OperationLimits { impl OperationLimits { // Checks for empty declarations - fn is_empty_optional_decl(spec_decl: &Option) -> bool { + fn is_empty_decl(spec_decl: Option<&SpecDecl>) -> bool { match spec_decl { None => true, - Some(decl) => Self::is_empty_decl(decl), - } - } - - fn is_empty_decl(spec_decl: &SpecDecl) -> bool { - match &spec_decl.body { - SpecBody::Gen(_) => true, - SpecBody::Impl(_, block) => block.stmts.is_empty(), + Some(decl) => match &decl.body { + SpecBody::Gen(_) => true, + SpecBody::Impl(_, block) => block.stmts.is_empty(), + }, } } // Empty operation means no code for body or specializations(implicit or explicit) fn is_empty_op(call_decl: &CallableDecl) -> bool { - Self::is_empty_decl(&call_decl.body) - && Self::is_empty_optional_decl(&call_decl.adj) - && Self::is_empty_optional_decl(&call_decl.ctl) - && Self::is_empty_optional_decl(&call_decl.ctl_adj) + 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()) } } From 286f92f4e29eed746322895887464cab6b93edef Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Fri, 31 May 2024 18:18:02 -0700 Subject: [PATCH 18/34] Minor edit --- compiler/qsc_linter/src/lints/hir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index d42faa323f..ff72a7fc87 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -39,7 +39,7 @@ impl OperationLimits { } } - // Empty operation means no code for body or specializations(implicit or explicit) + // Empty operation means no code for body and specializations(implicit or explicit) 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()) From ecce9e0d40123356445aaad70850e0bd382be7ff Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Mon, 3 Jun 2024 09:33:29 -0700 Subject: [PATCH 19/34] cleanup --- compiler/qsc_linter/src/tests.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index ea348f2fd1..1fdd27120e 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -178,8 +178,6 @@ fn needless_operation_non_empty_op_and_no_specialization() { ); } -// 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, #[test] fn needless_operation_non_empty_op_and_specialization() { check( @@ -260,7 +258,6 @@ fn needless_operation_partial_application() { 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); From cb0a26b1e115cb8f554ccc3748227da0e521c59d Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Mon, 3 Jun 2024 09:45:42 -0700 Subject: [PATCH 20/34] Add more test --- compiler/qsc_linter/src/tests.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 1fdd27120e..69f2f24ed4 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -161,6 +161,16 @@ fn needless_operation_lambda_operations() { ); } +#[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( From 2c32a5a23c99c0dd971384bd850c43f33b4d47d2 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Mon, 3 Jun 2024 09:47:22 -0700 Subject: [PATCH 21/34] edit --- compiler/qsc_linter/src/lints/hir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index ff72a7fc87..37f25bc329 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -39,7 +39,7 @@ impl OperationLimits { } } - // Empty operation means no code for body and specializations(implicit or explicit) + // Empty operation means no code for 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()) From 0d7b339e9ce1b4ad07ca4ab134acd225313eaa54 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Mon, 3 Jun 2024 10:55:32 -0700 Subject: [PATCH 22/34] Fix clippy warning --- compiler/qsc_linter/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 69f2f24ed4..22af12c198 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -165,9 +165,9 @@ fn needless_operation_lambda_operations() { fn needless_operation_no_lint_for_valid_lambda_operations() { check( &wrap_in_callable("let op = (q) => H(q);", CallableKind::Function), - &expect![[r#" + &expect![[r" [] - "#]], + "]], ); } From 127ad6a280f6478df3a71906eb18e34bda6a49a1 Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Mon, 3 Jun 2024 12:06:38 -0700 Subject: [PATCH 23/34] Update compiler/qsc_linter/src/lints/hir.rs Co-authored-by: orpuente-MS <156957451+orpuente-MS@users.noreply.github.com> --- compiler/qsc_linter/src/lints/hir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 37f25bc329..c74dd45582 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -15,7 +15,7 @@ declare_hir_lints! { (NeedlessOperation, LintLevel::Warn, "unnecessary operation declaration", "convert to function") } -// Helper to check if a operation has desired operation characteristics +// 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 From f01b9f12ca0089217052ee606c801c18b7c57129 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Mon, 3 Jun 2024 12:13:38 -0700 Subject: [PATCH 24/34] PR feedback --- compiler/qsc_linter/src/lints/hir.rs | 40 ++++++++++++++-------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index c74dd45582..7833ee7f3c 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -15,19 +15,19 @@ declare_hir_lints! { (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 +/// 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 OperationLimits { +struct IsQuantumOperation { // Operation Characteristics - op_char: bool, + is_op: bool, } -impl OperationLimits { +impl IsQuantumOperation { // Checks for empty declarations fn is_empty_decl(spec_decl: Option<&SpecDecl>) -> bool { match spec_decl { @@ -48,19 +48,19 @@ impl OperationLimits { } } -impl Visitor<'_> for OperationLimits { +impl Visitor<'_> for IsQuantumOperation { fn visit_callable_decl(&mut self, decl: &CallableDecl) { if Self::is_empty_op(decl) { - self.op_char = true; + self.is_op = true; } else { visit::walk_callable_decl(self, decl); } } fn visit_stmt(&mut self, stmt: &Stmt) { - if !self.op_char { + if !self.is_op { if let StmtKind::Qubit(..) = &stmt.kind { - self.op_char = true; + self.is_op = true; } else { visit::walk_stmt(self, stmt); } @@ -68,16 +68,16 @@ impl Visitor<'_> for OperationLimits { } fn visit_expr(&mut self, expr: &Expr) { - if !self.op_char { + if !self.is_op { match &expr.kind { ExprKind::Call(callee, _) => { if matches!(&callee.ty, Ty::Arrow(arrow) if arrow.kind == CallableKind::Operation) { - self.op_char = true; + self.is_op = true; } } ExprKind::Conjugate(..) | ExprKind::Repeat(..) => { - self.op_char = true; + self.is_op = true; } _ => { visit::walk_expr(self, expr); @@ -87,16 +87,16 @@ impl Visitor<'_> for OperationLimits { } } -// HIR Lint for `NeedlessOperation`, suggesting to use function -// We use `OperationLimits` helper to check if a operation has desired operation characteristics +/// 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 = OperationLimits::default(); + let mut op_limits = IsQuantumOperation::default(); op_limits.visit_callable_decl(decl); - if !op_limits.op_char { + if !op_limits.is_op { buffer.push(lint!(self, decl.span)); } } From e5bbcc30984702802b8b2902c4219be45e3efee1 Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Mon, 3 Jun 2024 12:56:48 -0700 Subject: [PATCH 25/34] Apply suggestions from code review Co-authored-by: orpuente-MS <156957451+orpuente-MS@users.noreply.github.com> --- compiler/qsc_linter/src/lints/hir.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 7833ee7f3c..d733e91db3 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -28,7 +28,7 @@ struct IsQuantumOperation { } impl IsQuantumOperation { - // Checks for empty declarations + /// Returns `true` if the declaration is empty. fn is_empty_decl(spec_decl: Option<&SpecDecl>) -> bool { match spec_decl { None => true, @@ -39,7 +39,8 @@ impl IsQuantumOperation { } } - // Empty operation means no code for body and specializations. + /// 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()) From dfc143d3ed30248320200477f0aa893fafb6991e Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Mon, 3 Jun 2024 12:57:28 -0700 Subject: [PATCH 26/34] Update compiler/qsc_linter/src/lints/hir.rs Co-authored-by: orpuente-MS <156957451+orpuente-MS@users.noreply.github.com> --- compiler/qsc_linter/src/lints/hir.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index d733e91db3..2e10416919 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -23,7 +23,8 @@ declare_hir_lints! { /// non-empty operations with no specializations, and no quantum operations: show lint, offer quickfix to convert to function #[derive(Default)] struct IsQuantumOperation { - // Operation Characteristics + /// This field is set to `true` after calling `Visitor::visit_callable_decl(...)` + /// if the operation satisfies the characteristics described above. is_op: bool, } From cc20abf5591367edd37b10b09fd36388f0a33daf Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Mon, 3 Jun 2024 21:56:39 -0700 Subject: [PATCH 27/34] Update samples/language/GettingStarted.qs --- samples/language/GettingStarted.qs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/language/GettingStarted.qs b/samples/language/GettingStarted.qs index 928d9f7123..7ede6f5a62 100644 --- a/samples/language/GettingStarted.qs +++ b/samples/language/GettingStarted.qs @@ -7,6 +7,6 @@ namespace MyQuantumProgram { @EntryPoint() operation Main() : Unit { - use q = Qubit(); + // TODO: Write your Q# code here. } } From 3944165a56aaf67453983f2d98ee34bb08214355 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Mon, 3 Jun 2024 22:06:26 -0700 Subject: [PATCH 28/34] Fix rust panic --- language_service/src/code_action.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/language_service/src/code_action.rs b/language_service/src/code_action.rs index 6a456f4cb1..43e26e3ade 100644 --- a/language_service/src/code_action.rs +++ b/language_service/src/code_action.rs @@ -8,7 +8,7 @@ use qsc::{ line_column::{Encoding, Range}, Span, }; -use qsc_linter::AstLint; +use qsc_linter::{AstLint, HirLint}; use crate::{ compilation::Compilation, @@ -87,8 +87,8 @@ fn quick_fixes( kind: Some(CodeActionKind::QuickFix), is_preferred: None, }), - LintKind::Ast(AstLint::DivisionByZero) => (), - LintKind::Hir(_) => todo!(), + LintKind::Ast(AstLint::DivisionByZero) + | LintKind::Hir(HirLint::NeedlessOperation) => (), } } } From 73eb008e438544dd974e001d6468fb44cd5a6aab Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Fri, 7 Jun 2024 21:13:10 -0700 Subject: [PATCH 29/34] Revert changes in typescript unit tests --- npm/qsharp/test/basics.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/qsharp/test/basics.js b/npm/qsharp/test/basics.js index 035b594830..0bc39c2302 100644 --- a/npm/qsharp/test/basics.js +++ b/npm/qsharp/test/basics.js @@ -650,14 +650,14 @@ test("language service in notebook", async () => { }); await languageService.updateNotebookDocument("notebook.ipynb", 1, {}, [ - { uri: "cell1", version: 1, code: "function Main() : Unit {}" }, + { uri: "cell1", version: 1, code: "operation Main() : Unit {}" }, { uri: "cell2", version: 1, code: "Foo()" }, ]); // Above document should have generated a resolve error. await languageService.updateNotebookDocument("notebook.ipynb", 2, {}, [ - { uri: "cell1", version: 2, code: "function Main() : Unit {}" }, + { uri: "cell1", version: 2, code: "operation Main() : Unit {}" }, { uri: "cell2", version: 2, code: "Main()" }, ]); From 6a24701a1816311502b16f7dafbb48f783b944d9 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Fri, 7 Jun 2024 21:30:26 -0700 Subject: [PATCH 30/34] revert rust tests --- language_service/src/state/tests.rs | 86 ++++++++++++++--------------- language_service/src/tests.rs | 10 ++-- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/language_service/src/state/tests.rs b/language_service/src/state/tests.rs index 963a69db85..cf3836c410 100644 --- a/language_service/src/state/tests.rs +++ b/language_service/src/state/tests.rs @@ -21,7 +21,7 @@ async fn no_error() { .update_document( "single/foo.qs", 1, - "namespace Foo { @EntryPoint() function Main() : Unit {} }", + "namespace Foo { @EntryPoint() operation Main() : Unit {} }", ) .await; @@ -80,7 +80,7 @@ async fn clear_error() { .update_document( "single/foo.qs", 2, - "namespace Foo { @EntryPoint() function Main() : Unit {} }", + "namespace Foo { @EntryPoint() operation Main() : Unit {} }", ) .await; @@ -109,7 +109,7 @@ async fn close_last_doc_in_project() { .update_document( "project/src/other_file.qs", 1, - "namespace Foo { @EntryPoint() function Main() : Unit {} }", + "namespace Foo { @EntryPoint() operation Main() : Unit {} }", ) .await; updater @@ -131,7 +131,7 @@ async fn close_last_doc_in_project() { "project/src/other_file.qs": OpenDocument { version: 1, compilation: "project/qsharp.json", - latest_str_content: "namespace Foo { @EntryPoint() function Main() : Unit {} }", + latest_str_content: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", }, } "#]], @@ -140,13 +140,13 @@ async fn close_last_doc_in_project() { sources: [ Source { name: "project/src/other_file.qs", - contents: "namespace Foo { @EntryPoint() function Main() : Unit {} }", + contents: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 58, + offset: 59, }, ], common_prefix: Some( @@ -172,8 +172,8 @@ async fn close_last_doc_in_project() { Slash, ), Span { - lo: 58, - hi: 58, + lo: 59, + hi: 59, }, ), ), @@ -603,7 +603,7 @@ fn notebook_document_no_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "function Main() : Unit {}"), + ("cell1", 1, "operation Main() : Unit {}"), ("cell2", 1, "Main()"), ] .into_iter(), @@ -626,7 +626,7 @@ fn notebook_document_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "function Main() : Unit {}"), + ("cell1", 1, "operation Main() : Unit {}"), ("cell2", 1, "Foo()"), ] .into_iter(), @@ -648,8 +648,8 @@ fn notebook_document_errors() { NotFound( "Foo", Span { - lo: 26, - hi: 29, + lo: 27, + hi: 30, }, ), ), @@ -661,8 +661,8 @@ fn notebook_document_errors() { Error( AmbiguousTy( Span { - lo: 26, - hi: 31, + lo: 27, + hi: 32, }, ), ), @@ -753,7 +753,7 @@ fn notebook_update_remove_cell_clears_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "function Main() : Unit {}"), + ("cell1", 1, "operation Main() : Unit {}"), ("cell2", 1, "Foo()"), ] .into_iter(), @@ -775,8 +775,8 @@ fn notebook_update_remove_cell_clears_errors() { NotFound( "Foo", Span { - lo: 26, - hi: 29, + lo: 27, + hi: 30, }, ), ), @@ -788,8 +788,8 @@ fn notebook_update_remove_cell_clears_errors() { Error( AmbiguousTy( Span { - lo: 26, - hi: 31, + lo: 27, + hi: 32, }, ), ), @@ -805,7 +805,7 @@ fn notebook_update_remove_cell_clears_errors() { updater.update_notebook_document( "notebook.ipynb", &NotebookMetadata::default(), - [("cell1", 1, "function Main() : Unit {}")].into_iter(), + [("cell1", 1, "operation Main() : Unit {}")].into_iter(), ); expect_errors( @@ -831,7 +831,7 @@ fn close_notebook_clears_errors() { "notebook.ipynb", &NotebookMetadata::default(), [ - ("cell1", 1, "function Main() : Unit {}"), + ("cell1", 1, "operation Main() : Unit {}"), ("cell2", 1, "Foo()"), ] .into_iter(), @@ -853,8 +853,8 @@ fn close_notebook_clears_errors() { NotFound( "Foo", Span { - lo: 26, - hi: 29, + lo: 27, + hi: 30, }, ), ), @@ -866,8 +866,8 @@ fn close_notebook_clears_errors() { Error( AmbiguousTy( Span { - lo: 26, - hi: 31, + lo: 27, + hi: 32, }, ), ), @@ -905,7 +905,7 @@ async fn update_doc_updates_project() { .update_document( "project/src/other_file.qs", 1, - "namespace Foo { @EntryPoint() function Main() : Unit {} }", + "namespace Foo { @EntryPoint() operation Main() : Unit {} }", ) .await; updater @@ -929,7 +929,7 @@ async fn update_doc_updates_project() { "project/src/other_file.qs": OpenDocument { version: 1, compilation: "project/qsharp.json", - latest_str_content: "namespace Foo { @EntryPoint() function Main() : Unit {} }", + latest_str_content: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", }, } "#]], @@ -938,13 +938,13 @@ async fn update_doc_updates_project() { sources: [ Source { name: "project/src/other_file.qs", - contents: "namespace Foo { @EntryPoint() function Main() : Unit {} }", + contents: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "namespace Foo { we should see this in the source }", - offset: 58, + offset: 59, }, ], common_prefix: Some( @@ -971,8 +971,8 @@ async fn update_doc_updates_project() { ), Ident, Span { - lo: 74, - hi: 76, + lo: 75, + hi: 77, }, ), ), @@ -1001,7 +1001,7 @@ async fn close_doc_prioritizes_fs() { .update_document( "project/src/other_file.qs", 1, - "namespace Foo { @EntryPoint() function Main() : Unit {} }", + "namespace Foo { @EntryPoint() operation Main() : Unit {} }", ) .await; updater @@ -1022,7 +1022,7 @@ async fn close_doc_prioritizes_fs() { "project/src/other_file.qs": OpenDocument { version: 1, compilation: "project/qsharp.json", - latest_str_content: "namespace Foo { @EntryPoint() function Main() : Unit {} }", + latest_str_content: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", }, } "#]], @@ -1031,13 +1031,13 @@ async fn close_doc_prioritizes_fs() { sources: [ Source { name: "project/src/other_file.qs", - contents: "namespace Foo { @EntryPoint() function Main() : Unit {} }", + contents: "namespace Foo { @EntryPoint() operation Main() : Unit {} }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 58, + offset: 59, }, ], common_prefix: Some( @@ -1063,8 +1063,8 @@ async fn close_doc_prioritizes_fs() { Slash, ), Span { - lo: 58, - hi: 58, + lo: 59, + hi: 59, }, ), ), @@ -1112,13 +1112,13 @@ async fn delete_manifest() { sources: [ Source { name: "project/src/other_file.qs", - contents: "// DISK CONTENTS\n namespace OtherFile { function Other() : Unit { } }", + contents: "// DISK CONTENTS\n namespace OtherFile { operation Other() : Unit { } }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 70, + offset: 71, }, ], common_prefix: Some( @@ -1197,13 +1197,13 @@ async fn delete_manifest_then_close() { sources: [ Source { name: "project/src/other_file.qs", - contents: "// DISK CONTENTS\n namespace OtherFile { function Other() : Unit { } }", + contents: "// DISK CONTENTS\n namespace OtherFile { operation Other() : Unit { } }", offset: 0, }, Source { name: "project/src/this_file.qs", contents: "// DISK CONTENTS\n namespace Foo { }", - offset: 70, + offset: 71, }, ], common_prefix: Some( @@ -1432,7 +1432,7 @@ async fn doc_switches_project_on_close() { #[tokio::test] async fn loading_lints_config_from_manifest() { - let this_file_qs = "namespace Foo { function Main() : Unit { let x = 5 / 0 + (2 ^ 4); } }"; + let this_file_qs = "namespace Foo { operation Main() : Unit { let x = 5 / 0 + (2 ^ 4); } }"; let fs = FsNode::Dir( [dir( "project", @@ -1738,7 +1738,7 @@ fn test_fs() -> FsNode { [ file( "other_file.qs", - "// DISK CONTENTS\n namespace OtherFile { function Other() : Unit { } }", + "// DISK CONTENTS\n namespace OtherFile { operation Other() : Unit { } }", ), file("this_file.qs", "// DISK CONTENTS\n namespace Foo { }"), ], diff --git a/language_service/src/tests.rs b/language_service/src/tests.rs index ae4d69fa8f..c89443db32 100644 --- a/language_service/src/tests.rs +++ b/language_service/src/tests.rs @@ -110,7 +110,7 @@ async fn single_document_update() { ls.update_document( "foo.qs", 1, - "namespace Foo { @EntryPoint() function Bar() : Unit {} }", + "namespace Foo { @EntryPoint() operation Bar() : Unit {} }", ); worker.apply_pending().await; @@ -135,7 +135,7 @@ async fn single_document_update() { sources: [ Source { name: "foo.qs", - contents: "namespace Foo { @EntryPoint() function Bar() : Unit {} }", + contents: "namespace Foo { @EntryPoint() operation Bar() : Unit {} }", offset: 0, }, ], @@ -191,13 +191,13 @@ async fn document_in_project() { sources: [ Source { name: "other_file.qs", - contents: "namespace OtherFile { function Other() : Unit {} }", + contents: "namespace OtherFile { operation Other() : Unit {} }", offset: 0, }, Source { name: "this_file.qs", contents: "namespace Foo { }", - offset: 51, + offset: 52, }, ], common_prefix: None, @@ -327,7 +327,7 @@ fn create_update_worker<'a>( tokio::spawn(ready(match file.as_str() { "other_file.qs" => ( Arc::from(file), - Arc::from("namespace OtherFile { function Other() : Unit {} }"), + Arc::from("namespace OtherFile { operation Other() : Unit {} }"), ), "this_file.qs" => (Arc::from(file), Arc::from("namespace Foo { }")), _ => panic!("unknown file"), From 368134e7ba5ad08fbf9771616ef1e74c58dd1af6 Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Fri, 7 Jun 2024 21:42:19 -0700 Subject: [PATCH 31/34] Update compiler/qsc_linter/src/lints/hir.rs Co-authored-by: Mine Starks <16928427+minestarks@users.noreply.github.com> --- compiler/qsc_linter/src/lints/hir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 2e10416919..8c6fb06067 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -12,7 +12,7 @@ use crate::linter::hir::declare_hir_lints; use super::lint; declare_hir_lints! { - (NeedlessOperation, LintLevel::Warn, "unnecessary operation declaration", "convert to function") + (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 From b93fe51e323f00bfad1b9018164ab53d9950b4a6 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Fri, 7 Jun 2024 22:07:21 -0700 Subject: [PATCH 32/34] update tests after updating message and help --- compiler/qsc_linter/src/tests.rs | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 22af12c198..6d93082c88 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -39,8 +39,8 @@ fn multiple_lints() { SrcLint { source: "operation RunProgram() : Unit {\n let x = ((1 + 2)) / 0;;;;\n }", level: Warn, - message: "unnecessary operation declaration", - help: "convert to function", + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", }, ] "#]], @@ -153,8 +153,8 @@ fn needless_operation_lambda_operations() { SrcLint { source: "(a) => a + 1", level: Warn, - message: "unnecessary operation declaration", - help: "convert to function", + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", }, ] "#]], @@ -180,8 +180,8 @@ fn needless_operation_non_empty_op_and_no_specialization() { SrcLint { source: "operation RunProgram() : Unit {\n let x = 2;\n }", level: Warn, - message: "unnecessary operation declaration", - help: "convert to function", + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", }, ] "#]], @@ -200,15 +200,15 @@ fn needless_operation_non_empty_op_and_specialization() { } "}, &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", - }, - ] - "#]], + [ + SrcLint { + source: "operation Run(target : Qubit) : Unit is Adj {\n body ... {\n Message(\"hi\");\n }\n adjoint self;\n}", + level: Warn, + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", + }, + ] + "#]], ); } @@ -258,8 +258,8 @@ fn needless_operation_partial_application() { SrcLint { source: "operation PartialApplication(q1 : Qubit) : Qubit => Unit {\n return PrepareBellState(q1, _);\n}", level: Warn, - message: "unnecessary operation declaration", - help: "convert to function", + message: "operation does not contain any quantum operations", + help: "this callable can be declared as a function instead", }, ] "#]], From ad1e5738b5ddac42c2216c0014c96b34afc72796 Mon Sep 17 00:00:00 2001 From: Manvi-Agrawal <40084144+Manvi-Agrawal@users.noreply.github.com> Date: Fri, 7 Jun 2024 22:18:23 -0700 Subject: [PATCH 33/34] Update compiler/qsc_linter/src/lints/hir.rs Co-authored-by: Mine Starks <16928427+minestarks@users.noreply.github.com> --- compiler/qsc_linter/src/lints/hir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index 8c6fb06067..55dac75813 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -99,7 +99,7 @@ impl HirLintPass for NeedlessOperation { op_limits.visit_callable_decl(decl); if !op_limits.is_op { - buffer.push(lint!(self, decl.span)); + buffer.push(lint!(self, decl.name.span)); } } } From 680cd476723912ae0b755a349e128081b1333e91 Mon Sep 17 00:00:00 2001 From: Manvi Agrawal Date: Fri, 7 Jun 2024 22:20:28 -0700 Subject: [PATCH 34/34] Fix tests --- compiler/qsc_linter/src/tests.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index 6d93082c88..317b4976a7 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -37,7 +37,7 @@ fn multiple_lints() { help: "remove the extra parentheses for clarity", }, SrcLint { - source: "operation RunProgram() : Unit {\n let x = ((1 + 2)) / 0;;;;\n }", + source: "RunProgram", level: Warn, message: "operation does not contain any quantum operations", help: "this callable can be declared as a function instead", @@ -151,7 +151,7 @@ fn needless_operation_lambda_operations() { &expect![[r#" [ SrcLint { - source: "(a) => a + 1", + source: "", level: Warn, message: "operation does not contain any quantum operations", help: "this callable can be declared as a function instead", @@ -178,7 +178,7 @@ fn needless_operation_non_empty_op_and_no_specialization() { &expect![[r#" [ SrcLint { - source: "operation RunProgram() : Unit {\n let x = 2;\n }", + source: "RunProgram", level: Warn, message: "operation does not contain any quantum operations", help: "this callable can be declared as a function instead", @@ -202,7 +202,7 @@ fn needless_operation_non_empty_op_and_specialization() { &expect![[r#" [ SrcLint { - source: "operation Run(target : Qubit) : Unit is Adj {\n body ... {\n Message(\"hi\");\n }\n adjoint self;\n}", + source: "Run", level: Warn, message: "operation does not contain any quantum operations", help: "this callable can be declared as a function instead", @@ -256,7 +256,7 @@ fn needless_operation_partial_application() { &expect![[r#" [ SrcLint { - source: "operation PartialApplication(q1 : Qubit) : Qubit => Unit {\n return PrepareBellState(q1, _);\n}", + source: "PartialApplication", level: Warn, message: "operation does not contain any quantum operations", help: "this callable can be declared as a function instead",