Skip to content

Commit

Permalink
feat(libflux): associate registry attributes to import statements (#5332
Browse files Browse the repository at this point in the history
)

This change retrieves the `@registry` attribute from the package and
associates those definitions with corresponding imports in the same
package.

This gets encoded in the semantic graph and the flatbuffer so it is
accessible in both Rust and Go.
  • Loading branch information
jsternberg committed Nov 9, 2022
1 parent 1db3400 commit 77b1d78
Show file tree
Hide file tree
Showing 17 changed files with 242 additions and 62 deletions.
3 changes: 2 additions & 1 deletion internal/fbsemantic/semantic.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ table PackageClause {
table ImportDeclaration {
loc:SourceLocation;
alias:Identifier;
path:StringLiteral;
registry:string;
path:string;
}

table SourceLocation {
Expand Down
24 changes: 15 additions & 9 deletions internal/fbsemantic/semantic_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion interpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func GetOption(ctx context.Context, pkg string, option string) (values.Value, bo
}

func (itrp *Interpreter) doImport(ctx context.Context, dec *semantic.ImportDeclaration, scope values.Scope, importer Importer) error {
path := dec.Path.Value
path := dec.Path
pkg, err := importer.ImportPackageObject(path)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion interpreter/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestAccessNestedImport(t *testing.T) {
},
Imports: []*semantic.ImportDeclaration{
{
Path: &semantic.StringLiteral{Value: "b"},
Path: "b",
},
},
Body: []semantic.Statement{
Expand Down
156 changes: 147 additions & 9 deletions libflux/flux-core/src/semantic/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ pub enum ErrorKind {
ExtraParameterRecord,
#[error("invalid duration, {0}")]
InvalidDuration(String),
#[error("path segment \"{0}\" is not allowed in import path")]
InvalidImportPathSegment(String),
#[error("import path {0} conflicts with registry definition with the same path")]
ImportRegistryConflict(String),
#[error("incorrect number of parameters for registry annotation")]
RegistryIncorrectNumberOfParameters,
#[error("registry parameters must be string literals")]
RegistryInvalidLiteralForParameters,
#[error("duplicate registry definition")]
RegistryDuplicateName,
}

impl AsDiagnostic for ErrorKind {
Expand Down Expand Up @@ -356,6 +366,7 @@ impl<'a> Symbols<'a> {

pub(crate) struct Converter<'a> {
symbols: Symbols<'a>,
registry_map: HashMap<String, String>,
errors: Errors<Error>,
config: &'a AnalyzerConfig,
}
Expand All @@ -364,6 +375,7 @@ impl<'a> Converter<'a> {
fn new(config: &'a AnalyzerConfig) -> Self {
Converter {
symbols: Symbols::new(None),
registry_map: HashMap::new(),
errors: Errors::new(),
config,
}
Expand All @@ -372,6 +384,7 @@ impl<'a> Converter<'a> {
pub(crate) fn with_env(env: &'a Environment, config: &'a AnalyzerConfig) -> Self {
Converter {
symbols: Symbols::new(Some(env)),
registry_map: HashMap::new(),
errors: Errors::new(),
config,
}
Expand All @@ -391,6 +404,7 @@ impl<'a> Converter<'a> {

pub(crate) fn convert_package(&mut self, pkg: &ast::Package) -> Package {
let package = pkg.package.clone();
self.collect_registry_definitions(pkg);

self.symbols.enter_scope();

Expand All @@ -409,6 +423,45 @@ impl<'a> Converter<'a> {
}
}

fn collect_registry_definitions(&mut self, pkg: &ast::Package) {
for file in &pkg.files {
if let Some(pkg_clause) = &file.package {
for attr in &pkg_clause.base.attributes {
if attr.params.len() != 2 {
let err = located(
attr.base.location.clone(),
ErrorKind::RegistryIncorrectNumberOfParameters,
);
self.errors.push(err);
continue;
}

match (&attr.params[0].value, &attr.params[1].value) {
(ast::Expression::StringLit(name), ast::Expression::StringLit(url)) => {
if self.registry_map.contains_key(name.value.as_str()) {
let err = located(
attr.base.location.clone(),
ErrorKind::RegistryDuplicateName,
);
self.errors.push(err);
continue;
}
self.registry_map
.insert(name.value.clone(), url.value.clone());
}
_ => {
let err = located(
attr.base.location.clone(),
ErrorKind::RegistryInvalidLiteralForParameters,
);
self.errors.push(err);
}
}
}
}
}
}

fn convert_file(&mut self, package_name: &str, file: &ast::File) -> File {
let package = self.convert_package_clause(file.package.as_ref());
let imports = file
Expand Down Expand Up @@ -450,16 +503,54 @@ impl<'a> Converter<'a> {
(id.name.clone(), Some(id))
}
};
let path = self.convert_string_literal(&imp.path);
let (registry, path) = self.convert_import_path(&imp.path);

ImportDeclaration {
loc: imp.base.location.clone(),
alias,
registry,
path,
import_symbol,
}
}

fn convert_import_path(&mut self, path: &ast::StringLit) -> (Option<String>, String) {
let path_str = path.value.as_str();
// Empty components or components completely composed of dots are just not allowed.
for p in path_str.split('/') {
if p.is_empty() || p == "." || p == ".." {
self.errors.push(located(
path.base.location.clone(),
ErrorKind::InvalidImportPathSegment(p.to_string()),
));
return (None, path_str.to_string());
}
}

// Determine which registry names match the path and
// return the longest one that matches.
let registry = self
.registry_map
.iter()
.filter(|(name, _)| path_str.starts_with(name.as_str()))
.max_by_key(|(name, _)| name.as_str().len());
if let Some((name, url)) = registry {
// At least one registry definition matched.
// Determine if it is valid.
let import_path = path_str.trim_start_matches(name.as_str());
// This logic works because we already checked for empty path segments above.
if !import_path.is_empty() && import_path.starts_with('/') {
return (Some(url.to_string()), import_path[1..].to_string());
}

self.errors.push(located(
path.base.location.clone(),
ErrorKind::ImportRegistryConflict(path_str.to_string()),
));
}
(None, path_str.to_string())
}

fn convert_statements(&mut self, package: &str, stmts: &[ast::Statement]) -> Vec<Statement> {
stmts
.iter()
Expand Down Expand Up @@ -1500,19 +1591,66 @@ mod tests {
imports: vec![
ImportDeclaration {
loc: b.location.clone(),
path: StringLit {
loc: b.location.clone(),
value: "path/foo".to_string(),
},
registry: None,
path: "path/foo".to_string(),
alias: None,
import_symbol: symbols["foo"].clone(),
},
ImportDeclaration {
loc: b.location.clone(),
path: StringLit {
loc: b.location.clone(),
value: "path/bar".to_string(),
},
registry: None,
path: "path/bar".to_string(),
alias: Some(Identifier {
loc: b.location,
name: symbols["b"].clone(),
}),
import_symbol: symbols["b"].clone(),
},
],
body: Vec::new(),
}],
};
assert_eq!(want, got);
}

#[test]
fn test_convert_imports_with_registry() {
let b = ast::BaseNode::default();
let pkg = parse_package(
r#"@registry("modules", "https://fluxlang.dev/modules")
package qux
import "path/foo"
import b "modules/path/bar"
"#,
);

let got = test_convert(pkg).unwrap();
let symbols = collect_symbols(&got);

let want = Package {
loc: b.location.clone(),
package: "qux".to_string(),
files: vec![File {
loc: b.location.clone(),
package: Some(PackageClause {
loc: b.location.clone(),
name: Identifier {
loc: b.location.clone(),
name: symbols["qux"].clone(),
},
}),
imports: vec![
ImportDeclaration {
loc: b.location.clone(),
registry: None,
path: "path/foo".to_string(),
alias: None,
import_symbol: symbols["foo"].clone(),
},
ImportDeclaration {
loc: b.location.clone(),
registry: Some("https://fluxlang.dev/modules".to_string()),
path: "path/bar".to_string(),
alias: Some(Identifier {
loc: b.location,
name: symbols["b"].clone(),
Expand Down
13 changes: 11 additions & 2 deletions libflux/flux-core/src/semantic/flatbuffers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,10 +841,19 @@ impl<'a, 'b> semantic::walk::Visitor<'_> for SerializingVisitor<'a, 'b> {
}
};

let path = v.pop_expr_with_kind(fbsemantic::Expression::StringLiteral);
let registry = imp
.registry
.as_ref()
.and_then(|registry| v.create_string(registry));
let path = v.create_string(imp.path.as_str());
let import = fbsemantic::ImportDeclaration::create(
v.builder,
&fbsemantic::ImportDeclarationArgs { loc, alias, path },
&fbsemantic::ImportDeclarationArgs {
loc,
alias,
registry,
path,
},
);
v.import_decls.push(import);
}
Expand Down

0 comments on commit 77b1d78

Please sign in to comment.