Skip to content

Commit

Permalink
feat: Add a syntax for describing label literals
Browse files Browse the repository at this point in the history
Adds the syntax and locks it behind the `LabelPolymorphism` feature flag.
(#4928 exists as the next step to allow users to actually opt-in to it).

Closes #4927
  • Loading branch information
Markus Westerlind committed Sep 7, 2022
1 parent 379de99 commit c4033da
Show file tree
Hide file tree
Showing 16 changed files with 213 additions and 70 deletions.
3 changes: 3 additions & 0 deletions cmd/flux/cmd/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ func (t *TestRunner) Gather(roots []string) error {
baseAST.Files[0].Name = file.path
}
tcidens, asts, err := testcase.Transform(ctx, baseAST, modules)
if strings.Contains(file.path, "string_test") {
fmt.Printf("AAAAAAAAAAAAAAAAAAAAAA %s %v %v\n", file.path, len(asts), err)
}
if err != nil {
return err
}
Expand Down
12 changes: 12 additions & 0 deletions docs/SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,18 @@ Examples:

The regular expression syntax is defined by [RE2](https://github.com/google/re2/wiki/Syntax).

#### Label literals

```flux
.mylabel
._value
."with spaces"
```

A label literal represents a "label" used to refer to specific record fields. They have two variants, where the `.` can
be followed by either an identifier or a string literal (allowing labels with characters that are not allowed in identifiers to be specified).

### Variables

A variable represents a storage location for a single value.
Expand Down
19 changes: 17 additions & 2 deletions libflux/flux-core/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ pub enum Expression {
Regexp(RegexpLit),
#[serde(rename = "PipeLiteral")]
PipeLit(PipeLit),
#[serde(rename = "LabelLiteral")]
Label(LabelLit),

#[serde(rename = "BadExpression")]
Bad(Box<BadExpr>),
Expand Down Expand Up @@ -220,6 +222,7 @@ impl Expression {
Expression::DateTime(wrapped) => &wrapped.base,
Expression::Regexp(wrapped) => &wrapped.base,
Expression::PipeLit(wrapped) => &wrapped.base,
Expression::Label(wrapped) => &wrapped.base,
Expression::Bad(wrapped) => &wrapped.base,
Expression::StringExpr(wrapped) => &wrapped.base,
Expression::Paren(wrapped) => &wrapped.base,
Expand Down Expand Up @@ -614,7 +617,7 @@ pub enum MonoType {
#[serde(rename = "FunctionType")]
Function(Box<FunctionType>),
#[serde(rename = "LabelType")]
Label(Box<StringLit>),
Label(Box<LabelLit>),
}

impl MonoType {
Expand Down Expand Up @@ -727,7 +730,7 @@ pub enum ParameterType {
monotype: MonoType,
// Default value for this parameter. Currently only string literals are supported
// (to allow default labels to be specified)
default: Option<StringLit>,
default: Option<LabelLit>,
},
Pipe {
#[serde(skip_serializing_if = "BaseNode::is_empty")]
Expand Down Expand Up @@ -1449,6 +1452,18 @@ pub struct UintLit {
pub value: u64,
}

/// LabelLit represents a label. Used to specify specific record fields.
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
#[allow(missing_docs)]
pub struct LabelLit {
#[serde(skip_serializing_if = "BaseNode::is_empty")]
#[serde(default)]
#[serde(flatten)]
pub base: BaseNode,

pub value: String,
}

struct U64Visitor;

impl<'de> Visitor<'de> for U64Visitor {
Expand Down
9 changes: 7 additions & 2 deletions libflux/flux-core/src/ast/walk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ pub enum Node<'a> {
RegexpLit(&'a RegexpLit),
#[display(fmt = "PipeLit")]
PipeLit(&'a PipeLit),
#[display(fmt = "LabelLit")]
LabelLit(&'a LabelLit),

#[display(fmt = "BadExpr")]
BadExpr(&'a BadExpr),
Expand Down Expand Up @@ -153,6 +155,7 @@ impl<'a> Node<'a> {
Node::DateTimeLit(n) => &n.base,
Node::RegexpLit(n) => &n.base,
Node::PipeLit(n) => &n.base,
Node::LabelLit(n) => &n.base,
Node::BadExpr(n) => &n.base,
Node::ExprStmt(n) => &n.base,
Node::OptionStmt(n) => &n.base,
Expand Down Expand Up @@ -203,6 +206,7 @@ impl<'a> Node<'a> {
Expression::DateTime(e) => Node::DateTimeLit(e),
Expression::Regexp(e) => Node::RegexpLit(e),
Expression::PipeLit(e) => Node::PipeLit(e),
Expression::Label(e) => Node::LabelLit(e),
Expression::Bad(e) => Node::BadExpr(e),
}
}
Expand Down Expand Up @@ -377,6 +381,7 @@ where
Node::DateTimeLit(_) => {}
Node::RegexpLit(_) => {}
Node::PipeLit(_) => {}
Node::LabelLit(_) => {}
Node::BadExpr(n) => {
if let Some(e) = &n.expression {
walk(v, Node::from_expr(e));
Expand Down Expand Up @@ -455,7 +460,7 @@ where

walk(v, Node::MonoType(&f.monotype));
}
MonoType::Label(lit) => walk(v, Node::StringLit(lit)),
MonoType::Label(_) => (),
},
Node::PropertyType(n) => {
walk(v, Node::from_property_key(&n.name));
Expand All @@ -475,7 +480,7 @@ where
walk(v, Node::Identifier(name));
walk(v, Node::MonoType(monotype));
if let Some(default) = default {
walk(v, Node::StringLit(default));
walk(v, Node::LabelLit(default));
}
}
ParameterType::Pipe { name, monotype, .. } => {
Expand Down
18 changes: 16 additions & 2 deletions libflux/flux-core/src/formatter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ impl<'doc> Formatter<'doc> {
Node::DateTimeLit(x) => self.format_date_time_literal(x),
Node::RegexpLit(x) => self.format_regexp_literal(x),
Node::PipeLit(x) => self.format_pipe_literal(x),
Node::LabelLit(x) => self.format_label_literal(x),
Node::ExprStmt(x) => self.format_expression_statement(x),
Node::OptionStmt(x) => self.format_option_statement(x),
Node::ReturnStmt(x) => self.format_return_statement(x),
Expand Down Expand Up @@ -601,7 +602,12 @@ impl<'doc> Formatter<'doc> {
self.format_monotype(&n.monotype),
]
}
ast::MonoType::Label(label) => self.format_string_literal(label),
ast::MonoType::Label(label) => docs![
arena,
self.format_comments(&label.base.comments),
".",
&label.value,
],
}
.group()
}
Expand Down Expand Up @@ -645,7 +651,7 @@ impl<'doc> Formatter<'doc> {
": ",
self.format_monotype(monotype),
match default {
Some(default) => docs![arena, " = ", self.format_string_literal(default)],
Some(default) => docs![arena, " = ", self.format_label_literal(default)],
None => arena.nil(),
}
]
Expand Down Expand Up @@ -1372,6 +1378,7 @@ impl<'doc> Formatter<'doc> {
ast::Expression::DateTime(expr) => self.format_date_time_literal(expr),
ast::Expression::Regexp(expr) => self.format_regexp_literal(expr),
ast::Expression::PipeLit(expr) => self.format_pipe_literal(expr),
ast::Expression::Label(expr) => self.format_label_literal(expr),
ast::Expression::Bad(expr) => {
self.err = Some(anyhow!("bad expression"));
arena.nil()
Expand Down Expand Up @@ -1612,6 +1619,12 @@ impl<'doc> Formatter<'doc> {
docs![arena, self.format_comments(&n.base.comments), "<-"]
}

fn format_label_literal(&mut self, n: &'doc ast::LabelLit) -> Doc<'doc> {
let arena = self.arena;

docs![arena, self.format_comments(&n.base.comments), ".", &n.value]
}

fn format_text_part(&mut self, n: &'doc ast::TextPart) -> Doc<'doc> {
let arena = self.arena;

Expand Down Expand Up @@ -2010,6 +2023,7 @@ fn starts_with_comment(n: Node) -> bool {
Node::DateTimeLit(n) => !n.base.comments.is_empty(),
Node::RegexpLit(n) => !n.base.comments.is_empty(),
Node::PipeLit(n) => !n.base.comments.is_empty(),
Node::LabelLit(n) => !n.base.comments.is_empty(),
Node::BadExpr(_) => false,
Node::ExprStmt(n) => starts_with_comment(Node::from_expr(&n.expression)),
Node::OptionStmt(n) => !n.base.comments.is_empty(),
Expand Down
18 changes: 17 additions & 1 deletion libflux/flux-core/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ impl<'input> Parser<'input> {
}
TokenType::LBrace => self.parse_record_type(),
TokenType::LParen => self.parse_function_type(),
TokenType::Dot => MonoType::Label(Box::new(self.parse_label_literal())),
TokenType::Ident if t.lit == "stream" => {
let start = self.expect(TokenType::Ident);
self.open(TokenType::LBrack, TokenType::RBrack);
Expand Down Expand Up @@ -656,7 +657,7 @@ impl<'input> Parser<'input> {

let default = if self.peek().tok == TokenType::Assign {
self.expect(TokenType::Assign);
Some(self.parse_string_literal())
Some(self.parse_label_literal())
} else {
None
};
Expand Down Expand Up @@ -1469,6 +1470,7 @@ impl<'input> Parser<'input> {
}
TokenType::LBrace => Expression::Object(Box::new(self.parse_object_literal())),
TokenType::LParen => self.parse_paren_expression(),
TokenType::Dot => Expression::Label(self.parse_label_literal()),
// We got a bad token, do not consume it, but use it in the message.
// Other methods will match BadExpr and consume the token if needed.
_ => {
Expand Down Expand Up @@ -1611,6 +1613,20 @@ impl<'input> Parser<'input> {
}
}
}

fn parse_label_literal(&mut self) -> LabelLit {
let dot = self.expect(TokenType::Dot);
let tok = self.expect_one_of(&[TokenType::Ident, TokenType::String]);

let base = self.base_node_from_tokens(&dot, &tok);
let value = match tok.tok {
TokenType::String => self.new_string_literal(tok).value,
_ => tok.lit,
};

LabelLit { base, value }
}

fn parse_regexp_literal(&mut self) -> RegexpLit {
let t = self.expect(TokenType::Regex);
let value = strconv::parse_regex(t.lit.as_str());
Expand Down
34 changes: 31 additions & 3 deletions libflux/flux-core/src/semantic/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::{
nodes::{Assignment, Expression, Statement},
walk,
walk::Node,
AnalyzerConfig, Feature,
},
};

Expand Down Expand Up @@ -39,6 +40,8 @@ pub enum ErrorKind {
DependentOptions(String, String),
/// TestCase still remains in semantic graph.
TestCase,
/// Emitted when labels are used without the label polymorphism feature
LabelWithoutFeature,
}

impl std::fmt::Display for ErrorKind {
Expand All @@ -63,6 +66,9 @@ impl std::fmt::Display for ErrorKind {
depender, dependee
)),
Self::TestCase => f.write_str("TestCase statement exists in semantic graph"),
Self::LabelWithoutFeature => f.write_str(
"Labels are currently experimental. (See the labelPolymorphism feature)",
),
}
}
}
Expand All @@ -88,13 +94,35 @@ impl AsDiagnostic for ErrorKind {
/// - Dependent options: options declared within the same package must not depend on one another.
///
/// If any of these errors are found, `check()` will return the first one it finds, and `Ok(())` otherwise.
pub fn check(pkg: &nodes::Package) -> Result<()> {
pub fn check(pkg: &nodes::Package, config: &AnalyzerConfig) -> Result<()> {
let opts = check_option_stmts(pkg)?;
check_vars(pkg, &opts)?;
check_labels(pkg, config)?;
check_option_dependencies(&opts)?;
check_testcase(pkg)
}

fn check_labels(pkg: &nodes::Package, config: &AnalyzerConfig) -> Result<()> {
let mut error = None;

if !config.features.contains(&Feature::LabelPolymorphism) {
walk::walk(
&mut |node| {
if let Node::LabelLit(lit) = node {
error = Some(located(lit.loc.clone(), ErrorKind::LabelWithoutFeature));
}
},
walk::Node::Package(pkg),
);

if let Some(err) = error {
return Err(err);
}
}

Ok(())
}

/// `check_option_stmts` checks that options are not reassigned within a package.
/// Note that options can only appear at file scope since the structure of the semantic
/// graph only allows expression statements, assignments and return statements inside function bodies.
Expand Down Expand Up @@ -375,7 +403,7 @@ mod tests {
Err(e) => panic!("{}", e),
Ok(pkg) => pkg,
};
if let Err(e) = check::check(&pkg) {
if let Err(e) = check::check(&pkg, &Default::default()) {
panic!("check failed: {}", e)
}
}
Expand All @@ -392,7 +420,7 @@ mod tests {
}
};

match check::check(&pkg) {
match check::check(&pkg, &Default::default()) {
Ok(()) => panic!(r#"expected error "{}", got no error"#, want_msg),
Err(e) => {
let got_msg = format!("{}", e);
Expand Down
12 changes: 8 additions & 4 deletions libflux/flux-core/src/semantic/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,9 @@ impl<'a> Converter<'a> {
name.name.clone(),
types::Argument {
typ: self.convert_monotype(monotype, tvars),
default: default.as_ref().map(|_| MonoType::STRING),
default: default.as_ref().map(|lit| {
MonoType::Label(types::Label::from(lit.value.as_str()))
}),
},
);
}
Expand Down Expand Up @@ -698,9 +700,7 @@ impl<'a> Converter<'a> {
r
}

ast::MonoType::Label(string_lit) => {
MonoType::Label(types::Label::from(string_lit.value.as_str()))
}
ast::MonoType::Label(lit) => MonoType::Label(types::Label::from(lit.value.as_str())),
}
}

Expand Down Expand Up @@ -835,6 +835,10 @@ impl<'a> Converter<'a> {
}
}
}
ast::Expression::Label(lit) => Expression::Label(LabelLit {
loc: lit.base.location.clone(),
value: types::Label::from(self.symbols.lookup_property_key(&lit.value)),
}),
ast::Expression::DateTime(lit) => {
Expression::DateTime(self.convert_date_time_literal(lit))
}
Expand Down
Loading

0 comments on commit c4033da

Please sign in to comment.