Skip to content

Commit

Permalink
fix(compiler): missing span information when bringing invalid directo…
Browse files Browse the repository at this point in the history
…ry (#6780)

When bringing an invalid directory no span information was shown so it was difficult to figure out where the error came from.

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
yoav-steinberg authored Jun 27, 2024
1 parent d599e44 commit d6d8969
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 35 deletions.
91 changes: 58 additions & 33 deletions libs/wingc/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,22 @@ pub fn parse_wing_project(
) -> Vec<Utf8PathBuf> {
// Parse the initial path (even if we have already seen it before)
let dependent_wing_paths = match init_path.is_dir() {
true => parse_wing_directory(&init_path, files, file_graph, tree_sitter_trees, asts),
true => parse_wing_directory(
&init_path,
&WingSpan::for_file(init_path.to_string()),
files,
file_graph,
tree_sitter_trees,
asts,
),
false => parse_wing_file(&init_path, init_text, files, file_graph, tree_sitter_trees, asts),
};

// Store a stack of files that still need parsing
let mut unparsed_files = dependent_wing_paths;

// Parse all remaining files in the project
while let Some(file_or_dir_path) = unparsed_files.pop() {
while let Some((file_or_dir_path, source_ref)) = unparsed_files.pop() {
// Skip files that we have already seen before (they should already be parsed)
if files.contains_file(&file_or_dir_path) {
assert!(
Expand All @@ -189,7 +196,14 @@ pub fn parse_wing_project(

// Parse the file or directory
let dependent_wing_paths = match file_or_dir_path.is_dir() {
true => parse_wing_directory(&file_or_dir_path, files, file_graph, tree_sitter_trees, asts),
true => parse_wing_directory(
&file_or_dir_path,
&source_ref,
files,
file_graph,
tree_sitter_trees,
asts,
),
false => parse_wing_file(&file_or_dir_path, None, files, file_graph, tree_sitter_trees, asts),
};

Expand Down Expand Up @@ -227,7 +241,7 @@ fn parse_wing_file(
file_graph: &mut FileGraph,
tree_sitter_trees: &mut IndexMap<Utf8PathBuf, tree_sitter::Tree>,
asts: &mut IndexMap<Utf8PathBuf, Scope>,
) -> Vec<Utf8PathBuf> {
) -> Vec<(Utf8PathBuf, WingSpan)> {
let source_text = match source_text {
Some(text) => text,
None => fs::read_to_string(source_path).expect("read_to_string call failed"),
Expand Down Expand Up @@ -258,7 +272,7 @@ fn parse_wing_file(
// Update our collections of trees and ASTs and our file graph
tree_sitter_trees.insert(source_path.to_owned(), tree_sitter_tree);
asts.insert(source_path.to_owned(), scope);
file_graph.update_file(source_path, &dependent_wing_paths);
file_graph.update_file(source_path, dependent_wing_paths.iter().map(|(path, _)| path));

dependent_wing_paths
}
Expand All @@ -285,40 +299,32 @@ fn contains_non_symbolic(str: &str) -> bool {
!re.is_match(str)
}

fn check_valid_wing_dir_name(dir_path: &Utf8Path) {
fn check_valid_wing_dir_name(dir_path: &Utf8Path) -> bool {
if contains_non_symbolic(dir_path.file_name().unwrap()) {
report_diagnostic(Diagnostic {
message: format!(
"Cannot bring Wing directories that contain invalid characters: \"{}\"",
dir_path.file_name().unwrap()
),
span: None,
annotations: vec![],
hints: vec![],
});
return false;
}
true
}

fn parse_wing_directory(
source_path: &Utf8Path,
source_ref: &WingSpan,
files: &mut Files,
file_graph: &mut FileGraph,
tree_sitter_trees: &mut IndexMap<Utf8PathBuf, tree_sitter::Tree>,
asts: &mut IndexMap<Utf8PathBuf, Scope>,
) -> Vec<Utf8PathBuf> {
) -> Vec<(Utf8PathBuf, WingSpan)> {
// Collect a list of all files and subdirectories in the directory
let mut files_and_dirs = Vec::new();

if source_path.is_dir() && !dir_contains_wing_file(&source_path) {
report_diagnostic(Diagnostic {
message: format!(
report_diagnostic(Diagnostic::new(
format!(
"Cannot explicitly bring directory with no Wing files \"{}\"",
source_path.file_name().unwrap()
),
span: None,
annotations: vec![],
hints: vec![],
})
source_ref,
))
}

for entry in fs::read_dir(&source_path).expect("read_dir call failed") {
Expand All @@ -336,9 +342,17 @@ fn parse_wing_directory(
{
// before we add the path, we need to check that directory names are valid
if path.is_dir() {
check_valid_wing_dir_name(&path);
if !check_valid_wing_dir_name(&path) {
report_diagnostic(Diagnostic::new(
format!(
"Cannot bring Wing directories that contain sub-directories with invalid characters: \"{}\"",
path.file_name().unwrap()
),
source_ref,
).hint("Directory names must start with a letter or underscore and contain only letters, numbers, and underscores."));
}
}
files_and_dirs.push(path);
files_and_dirs.push((path, source_ref.clone()));
}
}

Expand All @@ -356,7 +370,7 @@ fn parse_wing_directory(
files.update_file(&source_path, "".to_string());
tree_sitter_trees.insert(source_path.to_owned(), tree_sitter_tree);
asts.insert(source_path.to_owned(), scope);
file_graph.update_file(source_path, &dependent_wing_paths);
file_graph.update_file(source_path, dependent_wing_paths.iter().map(|(path, _)| path));

dependent_wing_paths
}
Expand All @@ -371,9 +385,9 @@ pub struct Parser<'a> {
in_json: RefCell<u64>,
is_in_mut_json: RefCell<bool>,
is_in_loop: RefCell<bool>,
/// Track all file paths that have been found while parsing the current file
/// Track all file paths (and where they appear in the source) that have been found while parsing the current file
/// These will need to be eventually parsed (or diagnostics will be reported if they don't exist)
referenced_wing_paths: RefCell<Vec<Utf8PathBuf>>,
referenced_wing_paths: RefCell<Vec<(Utf8PathBuf, WingSpan)>>,
}

struct ParseErrorBuilder<'s> {
Expand Down Expand Up @@ -419,7 +433,7 @@ impl<'s> Parser<'s> {
}
}

pub fn parse(self, root: &Node) -> (Scope, Vec<Utf8PathBuf>) {
pub fn parse(self, root: &Node) -> (Scope, Vec<(Utf8PathBuf, WingSpan)>) {
let scope = match root.kind() {
"source" => self.build_scope(&root, Phase::Preflight),
_ => Scope::empty(),
Expand Down Expand Up @@ -1017,7 +1031,10 @@ impl<'s> Parser<'s> {
);
}

self.referenced_wing_paths.borrow_mut().push(source_path.clone());
self
.referenced_wing_paths
.borrow_mut()
.push((source_path.clone(), module_name.span()));

// parse error if no alias is provided
let module = if let Some(alias) = alias {
Expand All @@ -1039,7 +1056,10 @@ impl<'s> Parser<'s> {

// case: directory
if source_path.is_dir() {
self.referenced_wing_paths.borrow_mut().push(source_path.clone());
self
.referenced_wing_paths
.borrow_mut()
.push((source_path.clone(), module_name.span()));

// parse error if no alias is provided
let module = if let Some(alias) = alias {
Expand Down Expand Up @@ -1088,7 +1108,10 @@ impl<'s> Parser<'s> {
if is_wing_library(&Utf8Path::new(&module_dir)) {
return if let Some(alias) = alias {
// make sure the Wing library is also parsed
self.referenced_wing_paths.borrow_mut().push(module_dir.clone());
self
.referenced_wing_paths
.borrow_mut()
.push((module_dir.clone(), module_name.span()));

Ok(StmtKind::Bring {
source: BringSource::WingLibrary(
Expand Down Expand Up @@ -1156,9 +1179,11 @@ impl<'s> Parser<'s> {
.err();
})?;

self.referenced_wing_paths.borrow_mut().push(module_dir.clone());
// make sure the trusted library is also parsed
self.referenced_wing_paths.borrow_mut().push(module_dir.clone());
self
.referenced_wing_paths
.borrow_mut()
.push((module_dir.clone(), module_name.span()));

Ok(StmtKind::Bring {
source: BringSource::TrustedModule(
Expand Down
20 changes: 18 additions & 2 deletions tools/hangar/__snapshots__/invalid.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,28 @@ Duration <DURATION>"

exports[`bring_invalid_dir.w 1`] = `
"error: Cannot explicitly bring directory with no Wing files "no_wing_files"
--> ../../../examples/tests/invalid/bring_invalid_dir.w:4:7
|
4 | bring "./subdir3/no_wing_files" as whatchaGonnaBring;
| ^^^^^^^^^^^^^^^^^^^^^^^^^


error: Cannot bring Wing directories that contain invalid characters: "in.valid"
error: Cannot bring Wing directories that contain sub-directories with invalid characters: "in.valid"
--> ../../../examples/tests/invalid/bring_invalid_dir.w:1:7
|
1 | bring "./subdir3" as wontWork;
| ^^^^^^^^^^^
|
= hint: Directory names must start with a letter or underscore and contain only letters, numbers, and underscores.


error: Cannot bring Wing directories that contain invalid characters: "not-valid"
error: Cannot bring Wing directories that contain sub-directories with invalid characters: "not-valid"
--> ../../../examples/tests/invalid/bring_invalid_dir.w:1:7
|
1 | bring "./subdir3" as wontWork;
| ^^^^^^^^^^^
|
= hint: Directory names must start with a letter or underscore and contain only letters, numbers, and underscores.



Expand Down

0 comments on commit d6d8969

Please sign in to comment.