diff --git a/crates/mun_hir/src/item_scope.rs b/crates/mun_hir/src/item_scope.rs index 0661ad691..a25bde428 100644 --- a/crates/mun_hir/src/item_scope.rs +++ b/crates/mun_hir/src/item_scope.rs @@ -17,8 +17,8 @@ pub(crate) enum ImportType { } /// A struct that holds information on which name was imported via a glob import. This information -/// is used by the `PackageDef` collector to keep track of duplicates so that this doesnt result in -/// a duplicate name error: +/// is used by the `PackageDef` collector to keep track of duplicates so that this doesn't result in +/// a duplicate name error; e.g. : /// ```mun /// use foo::{Foo, *}; /// ``` @@ -48,7 +48,7 @@ pub(crate) struct AddResolutionFromImportResult { /// Whether or not adding the resolution changed the item scope pub changed: bool, - /// Whether or not adding the resolution will overwrite and existing entry + /// Whether or not adding the resolution will overwrite an existing entry pub duplicate: bool, } @@ -85,7 +85,7 @@ impl ItemScope { } /// Adds a named item resolution into the scope. Returns true if adding the resolution changes - /// the scope or not. + /// the scope. pub(crate) fn add_resolution( &mut self, name: Name, @@ -109,7 +109,7 @@ impl ItemScope { } /// Adds a named item resolution into the scope which is the result of a `use` statement. - /// Returns true if adding the resolution changes the scope or not. + /// Returns true if adding the resolution changes the scope. pub(crate) fn add_resolution_from_import( &mut self, glob_imports: &mut PerNsGlobImports, @@ -134,7 +134,7 @@ impl ItemScope { match $def_import_type { // If this is a wildcard import, add it to the list of items we imported // via a glob. This information is stored so if we later explicitly - // import this type or value, it doesnt cause a conflict. + // import this type or value, it doesn't cause a conflict. ImportType::Glob => { $glob_imports.$field.insert($lookup.clone()); } diff --git a/crates/mun_hir/src/module_tree.rs b/crates/mun_hir/src/module_tree.rs index 869f383ae..480ca50f3 100644 --- a/crates/mun_hir/src/module_tree.rs +++ b/crates/mun_hir/src/module_tree.rs @@ -12,7 +12,7 @@ use std::sync::Arc; /// Represents the tree of modules of a package. /// -/// The `ModuleTree` is build by looking at all the source files of the source root of a package and +/// The `ModuleTree` is built by looking at all the source files of the source root of a package and /// creating a tree based on their relative paths. See the [`ModuleTree::module_tree_query`] method. /// When constructing the `ModuleTree` extra empty modules may be added for missing files. For /// instance for the relative path `foo/bar/baz.mun`, besides the module `foo::bar::baz` the modules diff --git a/crates/mun_hir/src/name_resolution/path_resolution.rs b/crates/mun_hir/src/name_resolution/path_resolution.rs index 709933844..c01d9937c 100644 --- a/crates/mun_hir/src/name_resolution/path_resolution.rs +++ b/crates/mun_hir/src/name_resolution/path_resolution.rs @@ -9,7 +9,7 @@ use std::iter::successors; /// Indicates whether or not any newly resolved import statements will actually change the outcome /// of an operation. This is useful to know if more iterations of an algorithm might be required, or -/// if its hopeless. +/// whether its hopeless. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ReachedFixedPoint { Yes, diff --git a/crates/mun_hir/src/package_defs.rs b/crates/mun_hir/src/package_defs.rs index 26bbc9ebe..ee9efddfd 100644 --- a/crates/mun_hir/src/package_defs.rs +++ b/crates/mun_hir/src/package_defs.rs @@ -55,12 +55,14 @@ mod diagnostics { use mun_syntax::ast::Use; use mun_syntax::{ast, AstPtr}; + /// A type of diagnostic that may be emitted during resolving all package definitions. #[derive(Debug, PartialEq, Eq)] enum DiagnosticKind { UnresolvedImport { ast: AstId, index: usize }, DuplicateImport { ast: AstId, index: usize }, } + /// A diagnostic that may be emitted during resolving all package definitions. #[derive(Debug, PartialEq, Eq)] pub(super) struct DefDiagnostic { /// The module that contains the diagnostic diff --git a/crates/mun_hir/src/package_defs/collector.rs b/crates/mun_hir/src/package_defs/collector.rs index b2ecb04ef..7d136b4a0 100644 --- a/crates/mun_hir/src/package_defs/collector.rs +++ b/crates/mun_hir/src/package_defs/collector.rs @@ -1,10 +1,9 @@ use super::PackageDefs; -use crate::item_scope::PerNsGlobImports; use crate::{ ids::ItemDefinitionId, ids::{FunctionLoc, Intern, StructLoc, TypeAliasLoc}, item_scope::ImportType, - item_scope::ItemScope, + item_scope::{ItemScope, PerNsGlobImports}, item_tree::{ self, Function, ItemTree, ItemTreeId, LocalItemTreeId, ModItem, Struct, StructDefKind, TypeAlias, @@ -18,13 +17,14 @@ use crate::{ }; use rustc_hash::FxHashMap; +/// Result of resolving an import statement #[derive(Copy, Clone, Debug, Eq, PartialEq)] -enum PartialResolvedImport { - /// None of any namespaces is resolved +enum PartiallyResolvedImport { + /// None of the namespaces are resolved Unresolved, /// One of namespaces is resolved. Indeterminate(PerNs<(ItemDefinitionId, Visibility)>), - /// All namespaces are resolved, OR it is came from other crate + /// All namespaces are resolved, OR it came from another crate Resolved(PerNs<(ItemDefinitionId, Visibility)>), } @@ -38,16 +38,17 @@ struct ImportResolution { resolution: PerNs<(ItemDefinitionId, Visibility)>, } -impl PartialResolvedImport { +impl PartiallyResolvedImport { fn namespaces(&self) -> PerNs<(ItemDefinitionId, Visibility)> { match self { - PartialResolvedImport::Unresolved => PerNs::none(), - PartialResolvedImport::Indeterminate(ns) => *ns, - PartialResolvedImport::Resolved(ns) => *ns, + PartiallyResolvedImport::Unresolved => PerNs::none(), + PartiallyResolvedImport::Indeterminate(ns) => *ns, + PartiallyResolvedImport::Resolved(ns) => *ns, } } } +/// Definition of a single import statement #[derive(Clone, Debug, Eq, PartialEq)] struct Import { /// The path of the import (e.g. foo::Bar). Note that group imports have been desugared, each @@ -92,7 +93,7 @@ struct ImportDirective { import: Import, /// The current status of the import. - status: PartialResolvedImport, + status: PartiallyResolvedImport, } pub(super) fn collect(db: &dyn DefDatabase, package_id: PackageId) -> PackageDefs { @@ -121,7 +122,7 @@ struct DefCollector<'db> { unresolved_imports: Vec, resolved_imports: Vec, - /// A mapping from local module to wildcard imports to other modules + /// A mapping from local module to wildcard imports of other modules glob_imports: FxHashMap)>>, @@ -135,7 +136,7 @@ impl<'db> DefCollector<'db> { // Collect all definitions in each module let module_tree = self.package_defs.module_tree.clone(); - // Start by collecting the definitions from all modules. This ensures that very every module + // Start by collecting the definitions from all modules. This ensures that, for every module, // all local definitions are accessible. This is the starting point for the import // resolution. collect_modules_recursive(self, module_tree.root, None); @@ -156,21 +157,21 @@ impl<'db> DefCollector<'db> { // Check the status of the import, if the import is still considered unresolved, try // again in the next round. match directive.status { - PartialResolvedImport::Indeterminate(_) => { + PartiallyResolvedImport::Indeterminate(_) => { self.record_resolved_import(&directive); - // FIXME: To avoid performance regression, we consider an imported resolved + // FIXME: To avoid performance regression, we consider an import resolved // if it is indeterminate (i.e not all namespace resolved). This might not // completely resolve correctly in the future if we can have values and // types with the same name. self.resolved_imports.push(directive); resolved_something = true; } - PartialResolvedImport::Resolved(_) => { + PartiallyResolvedImport::Resolved(_) => { self.record_resolved_import(&directive); self.resolved_imports.push(directive); resolved_something = true; } - PartialResolvedImport::Unresolved => { + PartiallyResolvedImport::Unresolved => { self.unresolved_imports.push(directive); } } @@ -238,27 +239,27 @@ impl<'db> DefCollector<'db> { } /// Given an import, try to resolve it. - fn resolve_import(&self, module_id: LocalModuleId, import: &Import) -> PartialResolvedImport { + fn resolve_import(&self, module_id: LocalModuleId, import: &Import) -> PartiallyResolvedImport { let res = self .package_defs .resolve_path_with_fixedpoint(self.db, module_id, &import.path); let def = res.resolved_def; if res.reached_fixedpoint == ReachedFixedPoint::No || def.is_none() { - return PartialResolvedImport::Unresolved; + return PartiallyResolvedImport::Unresolved; } if let Some(package) = res.package { if package != self.package_defs.module_tree.package { - return PartialResolvedImport::Resolved(def); + return PartiallyResolvedImport::Resolved(def); } } - // Check whether all namespace is resolved + // Check whether all namespaces have been resolved if def.take_types().is_some() && def.take_values().is_some() { - PartialResolvedImport::Resolved(def) + PartiallyResolvedImport::Resolved(def) } else { - PartialResolvedImport::Indeterminate(def) + PartiallyResolvedImport::Indeterminate(def) } } @@ -316,7 +317,7 @@ impl<'db> DefCollector<'db> { } } Some((_, _)) => { - // Happens when wildcard importing something other than a module. I guess its ok to do nothing here? + // Happens when wildcard importing something other than a module. I guess it's ok to do nothing here? } None => { // Happens if a wildcard import refers to something other than a type? @@ -383,14 +384,14 @@ impl<'db> DefCollector<'db> { let mut changed = false; for ImportResolution { name, resolution } in resolutions { - // TODO: Add an error if the visibility of the item does not allow exposing with the + // TODO(#309): Add an error if the visibility of the item does not allow exposing with the // import visibility. e.g.: // ```mun // //- foo.mun // pub(package) struct Foo; // // //- main.mun - // pub foo::Foo; // This is not allowed because Foo is only public for the package. + // pub foo::Foo; // This is not allowed because Foo is only public within the package. // ``` match name { @@ -526,7 +527,7 @@ impl<'a> ModCollectorContext<'a, '_> { self.def_collector.unresolved_imports.push(ImportDirective { module_id: self.module_id, import: Import::from_use(&self.item_tree, InFile::new(self.file_id, id)), - status: PartialResolvedImport::Unresolved, + status: PartiallyResolvedImport::Unresolved, }); None } diff --git a/crates/mun_hir/src/path.rs b/crates/mun_hir/src/path.rs index 9d185fcf0..b09f5dcab 100644 --- a/crates/mun_hir/src/path.rs +++ b/crates/mun_hir/src/path.rs @@ -16,6 +16,7 @@ pub enum PathKind { Package, } +/// A possible import alias e.g. `Foo as Bar` or `Foo as _`. #[derive(Debug, Clone, PartialEq, Eq)] pub enum ImportAlias { /// Unnamed alias, as in `use Foo as _;` @@ -77,7 +78,7 @@ impl Path { None } - /// Construct a path from its segments + /// Constructs a path from its segments. pub fn from_segments(kind: PathKind, segments: impl IntoIterator) -> Path { let segments = segments.into_iter().collect::>(); Path { kind, segments } diff --git a/crates/mun_hir/src/visibility.rs b/crates/mun_hir/src/visibility.rs index 5318007c5..e11ded978 100644 --- a/crates/mun_hir/src/visibility.rs +++ b/crates/mun_hir/src/visibility.rs @@ -58,7 +58,7 @@ pub enum Visibility { } impl Visibility { - /// Returns true if an item with this visibility is accessible from the module from the + /// Returns true if an item with this visibility is accessible from the module of the /// specified `PackageDefs`. pub(crate) fn is_visible_from_module_tree( self, diff --git a/crates/mun_syntax/src/parsing/grammar/declarations.rs b/crates/mun_syntax/src/parsing/grammar/declarations.rs index 6231f2c86..b06ef9f31 100644 --- a/crates/mun_syntax/src/parsing/grammar/declarations.rs +++ b/crates/mun_syntax/src/parsing/grammar/declarations.rs @@ -151,7 +151,7 @@ fn use_tree(p: &mut Parser, top_level: bool) { if top_level { p.error_recover(msg, DECLARATION_RECOVERY_SET); } else { - // if we are parsing a nested tree, we have to eat a token to main balanced `{}` + // if we are parsing a nested tree, we have to eat a token to remain balanced `{}` p.error_and_bump(msg); } return;