Apply suggestions from code review

Co-authored-by: Wodann <Wodann@users.noreply.github.com>

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());
                             }

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

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,

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<ImportDirective>,
     resolved_imports: Vec<ImportDirective>,
 
-    /// 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<LocalModuleId, Vec<(LocalModuleId, Visibility, ItemTreeId<item_tree::Import>)>>,
 
@@ -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 {

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<Item = Name>) -> Path {
         let segments = segments.into_iter().collect::<Vec<_>>();
         Path { kind, segments }

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,

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;