Skip to content

Commit

Permalink
Refactor: use LocatedDiagnostic instead of maps of Diagnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
@SpontanCombust
SpontanCombust committed May 14, 2024
1 parent e3fe1b6 commit d658782
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 95 deletions.
41 changes: 19 additions & 22 deletions crates/analysis/src/jobs/merge_symtabs.rs
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,28 @@
use std::collections::HashMap;
use abs_path::AbsPath;
use witcherscript_diagnostics::*;
use crate::model::collections::SymbolTable;


/// Key in the `diagnostics` map is a local path of the source file in the source tree
pub fn merge_symbol_tables(
target_symtab: &mut SymbolTable,
source_symtab: SymbolTable,
diagnostics: &mut HashMap<AbsPath, Vec<Diagnostic>>
diagnostics: &mut Vec<LocatedDiagnostic>
) {
for (local_source_path, errors) in target_symtab.merge(source_symtab) {
let abs_source_path = target_symtab.script_root().join(local_source_path).unwrap();

let errors_as_diags = errors.into_iter()
.map(|err| Diagnostic {
range: err.incoming_location.label_range,
kind: DiagnosticKind::SymbolNameTaken {
name: err.occupied_path.components().last().unwrap().name.to_string(),
precursor_range: err.occupied_location.as_ref().map(|loc| loc.label_range),
precursor_file_path: err.occupied_location.as_ref().map(|loc| target_symtab.script_root().join(&loc.local_source_path).unwrap())
}.into()
});

diagnostics
.entry(abs_source_path)
.or_default()
.extend(errors_as_diags);
}
diagnostics.extend(
target_symtab
.merge(source_symtab)
.into_iter()
.map(|err| {
LocatedDiagnostic {
path: err.incoming_location.abs_source_path.clone(),
diagnostic: Diagnostic {
range: err.incoming_location.label_range,
kind: DiagnosticKind::SymbolNameTaken {
name: err.occupied_path.components().last().unwrap().name.to_string(),
precursor_range: err.occupied_location.as_ref().map(|loc| loc.label_range),
precursor_file_path: err.occupied_location.as_ref().map(|loc| loc.abs_source_path.clone())
}
}
}
})
);
}
150 changes: 96 additions & 54 deletions crates/analysis/src/jobs/scan_symbols.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ pub fn scan_symbols(
doc: &ScriptDocument,
local_source_path: &Path,
scripts_root: &AbsPath,
symtab: &mut SymbolTable
) -> Vec<Diagnostic> {
symtab: &mut SymbolTable,
diagnostics: &mut Vec<LocatedDiagnostic>
) {
let mut visitor = SymbolScannerVisitor {
symtab,
doc,
local_source_path,
scripts_root,
diagnostics: Vec::new(),
diagnostics,
current_path: SymbolPathBuf::empty(),
current_param_ordinal: 0,
current_var_ordinal: 0
};

script.visit_nodes(&mut visitor);
visitor.diagnostics
}


Expand All @@ -40,7 +40,7 @@ struct SymbolScannerVisitor<'a> {
doc: &'a ScriptDocument,
local_source_path: &'a Path,
scripts_root: &'a AbsPath,
diagnostics: Vec<Diagnostic>,
diagnostics: &'a mut Vec<LocatedDiagnostic>,

current_path: SymbolPathBuf,
current_param_ordinal: usize,
Expand All @@ -58,13 +58,16 @@ impl SymbolScannerVisitor<'_> {
.map(|loc| (Some(self.scripts_root.join(loc.local_source_path).unwrap()), Some(loc.label_range)))
.unwrap_or((None, None));

self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::SymbolNameTaken {
name: err.occupied_path.components().last().unwrap().name.to_string(),
precursor_file_path,
precursor_range
}.into()
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::SymbolNameTaken {
name: err.occupied_path.components().last().unwrap().name.to_string(),
precursor_file_path,
precursor_range
}
}
});
}

Expand All @@ -78,9 +81,12 @@ impl SymbolScannerVisitor<'_> {
fn check_type_from_identifier(&mut self, n: IdentifierNode) -> BasicTypeSymbolPath {
let type_name = n.value(&self.doc);
if type_name == ArrayTypeSymbol::TYPE_NAME {
self.diagnostics.push(Diagnostic {
range: Range::new(n.range().end, n.range().end),
kind: DiagnosticKind::MissingTypeArg
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range: n.range(),
kind: DiagnosticKind::MissingTypeArg
}
});
} else {
return BasicTypeSymbolPath::new(&type_name);
Expand All @@ -100,9 +106,12 @@ impl SymbolScannerVisitor<'_> {
}
} else {
// since only array type takes type argument, all other uses of type arg are invalid
self.diagnostics.push(Diagnostic {
range: n.type_arg().unwrap().range(),
kind: DiagnosticKind::UnnecessaryTypeArg
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range: n.type_arg().unwrap().range(),
kind: DiagnosticKind::UnnecessaryTypeArg
}
});

return self.check_type_from_identifier(n.type_name()).into();
Expand Down Expand Up @@ -138,9 +147,12 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {

for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if !sym.specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -189,9 +201,12 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {

for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if !sym.specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -249,9 +264,12 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {

for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if !sym.specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -322,9 +340,12 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {

for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if !sym.specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -369,18 +390,24 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {
for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if matches!(spec, MemberFunctionSpecifier::AccessModifier(_)) {
if found_access_modif_before {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::MultipleAccessModifiers
})
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::MultipleAccessModifiers
}
});
}
found_access_modif_before = true;
}

if !sym.specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -445,9 +472,12 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {
let mut specifiers = SpecifierBitmask::new();
for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if !specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -477,18 +507,24 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {
for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if matches!(spec, MemberVarSpecifier::AccessModifier(_)) {
if found_access_modif_before {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::MultipleAccessModifiers
})
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::MultipleAccessModifiers
}
});
}
found_access_modif_before = true;
}

if !specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down Expand Up @@ -523,18 +559,24 @@ impl SyntaxNodeVisitor for SymbolScannerVisitor<'_> {
for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) {
if matches!(spec, AutobindSpecifier::AccessModifier(_)) {
if found_access_modif_before {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::MultipleAccessModifiers
})
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::MultipleAccessModifiers
}
});
}
found_access_modif_before = true;
}

if !sym.specifiers.insert(spec) {
self.diagnostics.push(Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
self.diagnostics.push(LocatedDiagnostic {
path: self.symtab.script_root().join(self.local_source_path).unwrap(),
diagnostic: Diagnostic {
range,
kind: DiagnosticKind::RepeatedSpecifier
}
});
}
}
Expand Down
12 changes: 4 additions & 8 deletions crates/analysis/src/model/collections/symbol_table.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,14 @@ impl SymbolTable {



pub(crate) fn merge(&mut self, mut other: Self) -> HashMap<PathBuf, Vec<MergeConflictError>> {
let mut errors = HashMap::new();
pub(crate) fn merge(&mut self, mut other: Self) -> Vec<MergeConflictError> {
let mut errors = Vec::new();
if other.is_empty() {
return errors;
}

let mut root_children_sympaths = Vec::new();
for (file_path, sympath_roots) in other.source_path_assocs {
let mut file_errors = Vec::new();

self.source_path_assocs.entry(file_path.clone())
.or_default();

Expand All @@ -174,7 +172,7 @@ impl SymbolTable {
if let Some(occupying_variant) = self.symbols.get(root) {
let occupying_sym = occupying_variant.as_dyn();
if !occupying_sym.path().has_missing() {
file_errors.push(MergeConflictError {
errors.push(MergeConflictError {
occupied_path: occupying_sym.path().to_sympath_buf(),
occupied_location: self.locate(root),
incoming_location: SymbolLocation {
Expand Down Expand Up @@ -220,7 +218,7 @@ impl SymbolTable {

let occupying_sym = occupying_variant.as_dyn();
if !occupying_sym.path().has_missing() {
file_errors.push(MergeConflictError {
errors.push(MergeConflictError {
occupied_path: occupying_sym.path().to_sympath_buf(),
occupied_location: self.locate(&incoming_sympath),
incoming_location: SymbolLocation {
Expand All @@ -241,8 +239,6 @@ impl SymbolTable {

root_children_sympaths.clear();
}

errors.insert(file_path.clone(), file_errors);
}

errors
Expand Down
Loading

0 comments on commit d658782

Please sign in to comment.