-
Notifications
You must be signed in to change notification settings - Fork 133
Test Explorer in VS Code Extension and @Test()
Attribute
#2059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 29 commits
b79c31e
1c09783
54eb209
e6a0941
0495a6d
eae1b4d
759036f
cec4bf9
ad7d3e2
d9f288d
9ec9f41
a03ed14
c11476a
b247c35
e75f65b
131a341
b52ad25
180368e
e960916
7d2aadc
ac8bfd7
c22d14d
5a28ef1
16a2aed
214097c
adbc4b2
5b6a1f8
7cee532
be64105
e34b7d2
fa1ca42
790c29b
9d0190c
38e0f4b
43f4e17
bcd6d34
5421cb2
804fb0b
ffab724
cca48b5
eea9581
540de0b
f171141
8255955
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -279,6 +279,52 @@ impl Display for Package { | |
} | ||
} | ||
|
||
impl Package { | ||
/// Returns a collection of the fully qualified names of any callables annotated with `@Test()` | ||
pub fn collect_test_callables(&self) -> std::result::Result<Vec<String>, String> { | ||
let items_with_test_attribute = self | ||
.items | ||
.iter() | ||
.filter(|(_, item)| item.attrs.iter().any(|attr| *attr == Attr::Test)); | ||
|
||
let callables = items_with_test_attribute | ||
.filter(|(_, item)| matches!(item.kind, ItemKind::Callable(_))); | ||
|
||
let callable_names = callables | ||
.filter_map(|(_, item)| -> Option<std::result::Result<String, String>> { | ||
if let ItemKind::Callable(callable) = &item.kind { | ||
if !callable.generics.is_empty() | ||
|| callable.input.kind != PatKind::Tuple(vec![]) | ||
{ | ||
return None; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So does this just silently drop a callable with the Test attribute if it happens to be generic? No warning or error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error is detected in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I saw those checks there. Just wondering the value of even testing for it here also, as the project is already in error from the other pass that detects these, and thus I assume shouldn't run anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It just prevents the test from showing up in the list. The pass doesn't filter out the callables from the HIR, because we prefer to preserve as much of the AST as possible for error recovery in passes. So even in an error state, the compilation does return some HIR for a good user experience. We can't let those invalid callables show up in the test explorer right? |
||
} | ||
// this is indeed a test callable, so let's grab its parent name | ||
let name = match item.parent { | ||
None => Default::default(), | ||
Some(parent_id) => { | ||
let parent_item = self | ||
.items | ||
.get(parent_id) | ||
.expect("Parent item did not exist in package"); | ||
if let ItemKind::Namespace(ns, _) = &parent_item.kind { | ||
format!("{}.{}", ns.name(), callable.name.name) | ||
} else { | ||
callable.name.name.to_string() | ||
} | ||
} | ||
}; | ||
|
||
Some(Ok(name)) | ||
} else { | ||
None | ||
} | ||
}) | ||
.collect::<std::result::Result<_, _>>()?; | ||
|
||
Ok(callable_names) | ||
} | ||
} | ||
|
||
/// An item. | ||
#[derive(Clone, Debug, PartialEq)] | ||
pub struct Item { | ||
|
@@ -1359,6 +1405,8 @@ pub enum Attr { | |
/// Indicates that an intrinsic callable is a reset. This means that the operation will be marked as | ||
/// "irreversible" in the generated QIR. | ||
Reset, | ||
/// Indicates that a callable is a test case. | ||
Test, | ||
} | ||
|
||
impl Attr { | ||
|
@@ -1376,6 +1424,7 @@ The `not` operator is also supported to negate the attribute, e.g. `not Adaptive | |
Attr::SimulatableIntrinsic => "Indicates that an item should be treated as an intrinsic callable for QIR code generation and any implementation should only be used during simulation.", | ||
Attr::Measurement => "Indicates that an intrinsic callable is a measurement. This means that the operation will be marked as \"irreversible\" in the generated QIR, and output Result types will be moved to the arguments.", | ||
Attr::Reset => "Indicates that an intrinsic callable is a reset. This means that the operation will be marked as \"irreversible\" in the generated QIR.", | ||
Attr::Test => "Indicates that a callable is a test case.", | ||
} | ||
} | ||
} | ||
|
@@ -1391,6 +1440,7 @@ impl FromStr for Attr { | |
"SimulatableIntrinsic" => Ok(Self::SimulatableIntrinsic), | ||
"Measurement" => Ok(Self::Measurement), | ||
"Reset" => Ok(Self::Reset), | ||
"Test" => Ok(Self::Test), | ||
_ => Err(()), | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT License. | ||
|
||
use miette::Diagnostic; | ||
use qsc_data_structures::span::Span; | ||
use qsc_hir::{hir::Attr, visit::Visitor}; | ||
use thiserror::Error; | ||
|
||
#[cfg(test)] | ||
mod tests; | ||
|
||
#[derive(Clone, Debug, Diagnostic, Error)] | ||
pub enum TestAttributeError { | ||
#[error("test callables cannot take arguments")] | ||
CallableHasParameters(#[label] Span), | ||
#[error("test callables cannot have type parameters")] | ||
CallableHasTypeParameters(#[label] Span), | ||
} | ||
|
||
pub(crate) fn validate_test_attributes( | ||
package: &mut qsc_hir::hir::Package, | ||
) -> Vec<TestAttributeError> { | ||
let mut validator = TestAttributeValidator { errors: Vec::new() }; | ||
validator.visit_package(package); | ||
validator.errors | ||
} | ||
|
||
struct TestAttributeValidator { | ||
errors: Vec<TestAttributeError>, | ||
} | ||
|
||
impl<'a> Visitor<'a> for TestAttributeValidator { | ||
fn visit_callable_decl(&mut self, decl: &'a qsc_hir::hir::CallableDecl) { | ||
if decl.attrs.iter().any(|attr| matches!(attr, Attr::Test)) { | ||
if !decl.generics.is_empty() { | ||
self.errors | ||
.push(TestAttributeError::CallableHasTypeParameters( | ||
decl.name.span, | ||
)); | ||
} | ||
if decl.input.ty != qsc_hir::ty::Ty::UNIT { | ||
self.errors | ||
.push(TestAttributeError::CallableHasParameters(decl.name.span)); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT License. | ||
|
||
use expect_test::{expect, Expect}; | ||
use indoc::indoc; | ||
use qsc_data_structures::{language_features::LanguageFeatures, target::TargetCapabilityFlags}; | ||
use qsc_frontend::compile::{self, compile, PackageStore, SourceMap}; | ||
use qsc_hir::{validate::Validator, visit::Visitor}; | ||
|
||
use crate::test_attribute::validate_test_attributes; | ||
|
||
fn check(file: &str, expect: &Expect) { | ||
let store = PackageStore::new(compile::core()); | ||
let sources = SourceMap::new([("test".into(), file.into())], None); | ||
let mut unit = compile( | ||
&store, | ||
&[], | ||
sources, | ||
TargetCapabilityFlags::all(), | ||
LanguageFeatures::default(), | ||
); | ||
assert!(unit.errors.is_empty(), "{:?}", unit.errors); | ||
|
||
let errors = validate_test_attributes(&mut unit.package); | ||
Validator::default().visit_package(&unit.package); | ||
if errors.is_empty() { | ||
expect.assert_eq(&unit.package.to_string()); | ||
sezna marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
expect.assert_debug_eq(&errors); | ||
} | ||
} | ||
|
||
#[test] | ||
fn callable_cant_have_params() { | ||
check( | ||
indoc! {" | ||
namespace test { | ||
@Test() | ||
operation A(q : Qubit) : Unit { | ||
|
||
} | ||
} | ||
"}, | ||
&expect![[r#" | ||
[ | ||
CallableHasParameters( | ||
Span { | ||
lo: 43, | ||
hi: 44, | ||
}, | ||
), | ||
] | ||
"#]], | ||
); | ||
} | ||
|
||
#[test] | ||
fn callable_cant_have_type_params() { | ||
check( | ||
indoc! {" | ||
namespace test { | ||
@Test() | ||
operation A<'T>() : Unit { | ||
|
||
} | ||
} | ||
"}, | ||
&expect![[r#" | ||
[ | ||
CallableHasTypeParameters( | ||
Span { | ||
lo: 43, | ||
hi: 44, | ||
}, | ||
), | ||
] | ||
"#]], | ||
); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.