Skip to content

Commit

Permalink
fix(testcase): makes testcase a semantic check instead of an error (#…
Browse files Browse the repository at this point in the history 
…4566)

Now the AST -> Semantic convert step does not error on testcase
statements, rather an explicit semantic check is used to catch remaining
testcase statements. The TestCase semantic node behaves like a partial
file where it has a statement list instead of a block. A block is an
expression that evaluates to a value which is not appropriate for a
testcase.

Fixes influxdata/flux-lsp#403 because the LSP
already skips semantic checks and so it doesn't report a diagnostic
about a testcase statement.
  • Loading branch information
@nathanielc
nathanielc authored Mar 17, 2022
1 parent d8ccde2 commit a3a82bc
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 18 deletions.
53 changes: 52 additions & 1 deletion libflux/flux-core/src/semantic/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub enum ErrorKind {
VarReassignOption(String),
/// An option depends on another option declared in the same package.
DependentOptions(String, String),
/// TestCase still remains in semantic graph.
TestCase,
}

impl std::fmt::Display for ErrorKind {
Expand All @@ -60,6 +62,7 @@ impl std::fmt::Display for ErrorKind {
r#"option "{}" depends on option "{}", which is defined in the same package"#,
depender, dependee
)),
Self::TestCase => f.write_str("TestCase statement exists in semantic graph"),
}
}
}
Expand Down Expand Up @@ -88,7 +91,8 @@ impl AsDiagnostic for ErrorKind {
pub fn check(pkg: &nodes::Package) -> Result<()> {
let opts = check_option_stmts(pkg)?;
check_vars(pkg, &opts)?;
check_option_dependencies(&opts)
check_option_dependencies(&opts)?;
check_testcase(pkg)
}

/// `check_option_stmts` checks that options are not reassigned within a package.
Expand Down Expand Up @@ -300,6 +304,37 @@ impl<'a> walk::Visitor<'a> for OptionDepVisitor<'a> {
}
}

/// `check_testcase()` checks that no TestCaseStmt still exist in the semantic graph.
fn check_testcase(pkg: & nodes::Package) -> Result<()> {
let mut v = TestCaseVisitor { err: None };
walk::walk(&mut v, walk::Node::Package(pkg));
match v.err {
Some(e) => Err(e),
None => Ok(()),
}
}

struct TestCaseVisitor {
err: Option<Error>,
}

impl<'a> walk::Visitor<'a> for TestCaseVisitor {
fn visit(&mut self, node: Node<'a>) -> bool {
if self.err.is_some() {
return false;
}
match node {
walk::Node::TestCaseStmt(s) => {
self.err = Some(located(s.loc.clone(), ErrorKind::TestCase));
false
}
walk::Node::Package(_) => true,
walk::Node::File(_) => true,
_ => false,
}
}
}

#[cfg(test)]
mod tests {
use anyhow::Result;
Expand Down Expand Up @@ -858,4 +893,20 @@ mod tests {
"#,
]);
}
#[test]
fn test_testcase() {
// testcase statement
check_fail(
vec![
r#"
package foo
testcase x {
y = 1
}
"#,
],
"file_0.flux@4:21-6:22: TestCase statement exists in semantic graph",
);
}
}
22 changes: 17 additions & 5 deletions libflux/flux-core/src/semantic/convert.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ pub type Error = Located<ErrorKind>;
#[derive(Error, Debug, PartialEq)]
#[allow(missing_docs)]
pub enum ErrorKind {
#[error("TestCase is not supported in semantic analysis")]
TestCase,
#[error("invalid named type {0}")]
InvalidNamedType(String),
#[error("function types can have at most one pipe parameter")]
Expand Down Expand Up @@ -423,9 +421,7 @@ impl<'a> Converter<'a> {
}
ast::Statement::Test(s) => Statement::Test(Box::new(self.convert_test_statement(s))),
ast::Statement::TestCase(s) => {
self.errors
.push(located(s.base.location.clone(), ErrorKind::TestCase));
Statement::Error(s.base.location.clone())
Statement::TestCase(Box::new(self.convert_testcase(package, s)))
}
ast::Statement::Expr(s) => Statement::Expr(self.convert_expression_statement(s)),
ast::Statement::Return(s) => Statement::Return(self.convert_return_statement(s)),
Expand Down Expand Up @@ -463,6 +459,22 @@ impl<'a> Converter<'a> {
typ_expr: self.convert_polytype(&stmt.ty),
}
}
fn convert_testcase(&mut self, package: &str, stmt: &ast::TestCaseStmt) -> TestCaseStmt {
TestCaseStmt {
loc: stmt.base.location.clone(),
id: self.convert_identifier(&stmt.id),
extends: stmt
.extends
.as_ref()
.map(|e| self.convert_string_literal(e)),
body: stmt
.block
.body
.iter()
.map(|s| self.convert_statement(package, s))
.collect(),
}
}

fn convert_builtintype(&mut self, basic: &ast::NamedType) -> Result<BuiltinType> {
Ok(match basic.name.name.as_str() {
Expand Down
3 changes: 2 additions & 1 deletion libflux/flux-core/src/semantic/formatter/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ impl Formatter {
self.write_string("testcase ");
self.format_node(&walk::Node::Identifier(&n.id));
self.write_rune(' ');
self.format_node(&walk::Node::Block(&n.block));
// format the testcase statements
self.format_statement_list(&n.body);
}

fn format_assignment(&mut self, n: &semantic::nodes::Assignment) {
Expand Down
19 changes: 16 additions & 3 deletions libflux/flux-core/src/semantic/nodes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,15 +637,28 @@ impl TestStmt {
pub struct TestCaseStmt {
pub loc: ast::SourceLocation,
pub id: Identifier,
pub block: Block,
pub extends: Option<StringLit>,
pub body: Vec<Statement>,
}

impl TestCaseStmt {
fn infer(&mut self, infer: &mut InferState<'_, '_>) -> Result {
self.block.infer(infer)
for node in &mut self.body {
match node {
Statement::Builtin(stmt) => stmt.infer(infer)?,
Statement::Variable(stmt) => stmt.infer(infer)?,
Statement::Option(stmt) => stmt.infer(infer)?,
Statement::Expr(stmt) => stmt.infer(infer)?,
Statement::Test(stmt) => stmt.infer(infer)?,
Statement::TestCase(stmt) => stmt.infer(infer)?,
Statement::Return(stmt) => infer.error(stmt.loc.clone(), ErrorKind::InvalidReturn),
Statement::Error(_) => (),
}
}
Ok(())
}
fn apply(mut self, sub: &Substitution) -> Self {
self.block = self.block.apply(sub);
self.body = self.body.into_iter().map(|stmt| stmt.apply(sub)).collect();
self
}
}
Expand Down
7 changes: 6 additions & 1 deletion libflux/flux-core/src/semantic/walk/_walk.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,12 @@ where
}
Node::TestCaseStmt(n) => {
walk(v, Node::Identifier(&n.id));
walk(v, Node::Block(&n.block));
if let Some(e) = &n.extends {
walk(v, Node::StringLit(e));
}
for stmt in n.body.iter() {
walk(v, Node::from_stmt(stmt));
}
}
Node::BuiltinStmt(n) => {
walk(v, Node::Identifier(&n.id));
Expand Down
7 changes: 6 additions & 1 deletion libflux/flux-core/src/semantic/walk/walk_mut.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,12 @@ where
}
NodeMut::TestCaseStmt(ref mut n) => {
walk_mut(v, &mut NodeMut::Identifier(&mut n.id));
walk_mut(v, &mut NodeMut::Block(&mut n.block));
if let Some(e) = n.extends.as_mut() {
walk_mut(v, &mut NodeMut::StringLit(e));
}
for stmt in n.body.iter_mut() {
walk_mut(v, &mut NodeMut::from_stmt(stmt));
}
}
NodeMut::BuiltinStmt(ref mut n) => {
walk_mut(v, &mut NodeMut::Identifier(&mut n.id));
Expand Down
12 changes: 6 additions & 6 deletions libflux/go/libflux/buildinfo.gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,25 @@ var sourceHashes = map[string]string{
"libflux/flux-core/src/scanner/unicode.rl": "f923f3b385ddfa65c74427b11971785fc25ea806ca03d547045de808e16ef9a1",
"libflux/flux-core/src/scanner/unicode.rl.COPYING": "6cf2d5d26d52772ded8a5f0813f49f83dfa76006c5f398713be3854fe7bc4c7e",
"libflux/flux-core/src/semantic/bootstrap.rs": "53c53083f78bb7dc21bb4f84eccfad46c405e4aed9a3302d698137ac1e5d6f31",
"libflux/flux-core/src/semantic/check.rs": "88ce41e7da5ddb064f53d41bf8890aea345fe99bf088872189b94c806bd5e3a1",
"libflux/flux-core/src/semantic/convert.rs": "73e990b8cf6e0cdb834821ad3a9d8270a42ffbeade411766df08e982c4b1eed7",
"libflux/flux-core/src/semantic/check.rs": "682c484aaed01a4d0ba965bd535ed1b78a6362d996913469d09d83930e347ebf",
"libflux/flux-core/src/semantic/convert.rs": "8a46600d2cbf0aa17573608062b8dfdb0e5620353e6dbb7e0d0811d9b4bc8b88",
"libflux/flux-core/src/semantic/env.rs": "802257307bbe18137cd495ba450838dbca1ac9ae34da930b1822374bf2b33739",
"libflux/flux-core/src/semantic/flatbuffers/mod.rs": "81874a83260d0570a7c1835da0ee0e80cc2b12c315baa73c52a1f225224bfe87",
"libflux/flux-core/src/semantic/flatbuffers/semantic_generated.rs": "0f54e652b45b1515c71160a555200a062f9c3c5a681a14f7a0ac36cc19dd4e6f",
"libflux/flux-core/src/semantic/flatbuffers/types.rs": "c2ee84a606fa2469f1bdf7d6e5f94f63a63e86523c1d5f3d6bb4c7214d4d5308",
"libflux/flux-core/src/semantic/formatter/mod.rs": "838dd1fff3bac5370738878401dbae8b2fe91cc0f6b8c42fc75dbe51a81fe4d9",
"libflux/flux-core/src/semantic/formatter/mod.rs": "112f3cbf7023356cf8afc8a00c89584102c6b0745f55d426c9e2c187fbff159e",
"libflux/flux-core/src/semantic/fresh.rs": "3fa4e79772f62f024fee4dc8f7b5d672c437187d3153d9ab64a16f4bffb0fb50",
"libflux/flux-core/src/semantic/fs.rs": "f7f609bc8149769d99b737150e184a2d54029c0b768365dbcf08ff193b0e1f6f",
"libflux/flux-core/src/semantic/import.rs": "73adc35c735c317af0fa7625e77a58f6824667d8e59f35c7d05687bd02d79a84",
"libflux/flux-core/src/semantic/infer.rs": "a9cf793d640ece528f702cc850c031ddf7c003f0b24591d77165a66e1a164d69",
"libflux/flux-core/src/semantic/mod.rs": "7633dfe325c9db6fdc1e835c70ac1d00f5d2aaa71a66b19523e4044833f39c99",
"libflux/flux-core/src/semantic/nodes.rs": "7c00929d44b2182c9cd3cc96605a13468e56c42d4f42468c3e51317cca58cf29",
"libflux/flux-core/src/semantic/nodes.rs": "8ca1a16a4f6ebf83a3e3aa4fc487020d2e4fadd0c478114f70f96f62e0ccc628",
"libflux/flux-core/src/semantic/sub.rs": "05cf04fa0dd1bb04a85c249a5e65dcf5635fe3e49ef48e4620378a31d26d798f",
"libflux/flux-core/src/semantic/types.rs": "4bde7fb6650d34926b543d94eb6ed7665d896c1eca0a658ce6e3847caf891868",
"libflux/flux-core/src/semantic/walk/_walk.rs": "03a862d35eac8b74cdc7fae0a91ae155859ac02e1b32aa5d503f523188fc5fde",
"libflux/flux-core/src/semantic/walk/_walk.rs": "c05238896b10128d25a02aee8650b6fa2ea497dbb72126a1d3b554fb55d8f727",
"libflux/flux-core/src/semantic/walk/mod.rs": "2d44af6df22c6b93f5155c15cffd28be940f45668883c9b2d4a1184a72a68c68",
"libflux/flux-core/src/semantic/walk/test_utils.rs": "b980587707038c420d16b99f99587e69b71f02c32277db37c7e4a094d32f2b38",
"libflux/flux-core/src/semantic/walk/walk_mut.rs": "0d2482aeb8d270fdf09627577f921e841ee48803e4c5cc0c5ed3a88bc7709a67",
"libflux/flux-core/src/semantic/walk/walk_mut.rs": "536ce17fd57a64fd579ce7330520d395957ff32d61b4e542053d2d8b005dd50e",
"libflux/flux/Cargo.toml": "a2e64e0276b020bd66b9ae958343fed39ba838e723d101c81a415524e093945b",
"libflux/flux/FLUXDOC.md": "92e6dd8043bd87b4924e09aa28fb5346630aee1214de28ea2c8fc0687cad0785",
"libflux/flux/build.rs": "c3257f0014596c8f41396a115ffef11413aebd52006ec5fd5b7fc81cbe82ebd0",
Expand Down

0 comments on commit a3a82bc

Please sign in to comment.