From 63c257a491f334ff1c6d4e0c841899831c988952 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 26 Feb 2021 15:34:39 +0100 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Wodann --- crates/mun_hir/src/item_scope.rs | 10 +++++----- crates/mun_hir/src/module_tree.rs | 2 +- .../src/name_resolution/path_resolution.rs | 2 +- crates/mun_hir/src/package_defs/collector.rs | 16 ++++++++-------- crates/mun_hir/src/path.rs | 2 +- crates/mun_hir/src/visibility.rs | 2 +- .../src/parsing/grammar/declarations.rs | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/mun_hir/src/item_scope.rs b/crates/mun_hir/src/item_scope.rs index b4f2f4fda..a25bde428 100644 --- a/crates/mun_hir/src/item_scope.rs +++ b/crates/mun_hir/src/item_scope.rs @@ -18,7 +18,7 @@ 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 doesn't result in -/// a duplicate name error: +/// 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/collector.rs b/crates/mun_hir/src/package_defs/collector.rs index b2ecb04ef..5911abe82 100644 --- a/crates/mun_hir/src/package_defs/collector.rs +++ b/crates/mun_hir/src/package_defs/collector.rs @@ -20,11 +20,11 @@ use rustc_hash::FxHashMap; #[derive(Copy, Clone, Debug, Eq, PartialEq)] enum PartialResolvedImport { - /// None of any namespaces is resolved + /// 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)>), } @@ -121,7 +121,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 +135,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); @@ -158,7 +158,7 @@ impl<'db> DefCollector<'db> { match directive.status { PartialResolvedImport::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. @@ -254,7 +254,7 @@ impl<'db> DefCollector<'db> { } } - // 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) } else { @@ -316,7 +316,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? @@ -390,7 +390,7 @@ impl<'db> DefCollector<'db> { // 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 { diff --git a/crates/mun_hir/src/path.rs b/crates/mun_hir/src/path.rs index 9d185fcf0..ebd3d3c7f 100644 --- a/crates/mun_hir/src/path.rs +++ b/crates/mun_hir/src/path.rs @@ -77,7 +77,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;