Skip to content

Commit

Permalink
fix: increase code coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
baszalmstra committed Jan 1, 2024
1 parent d41163a commit 9290fd5
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 29 deletions.
16 changes: 11 additions & 5 deletions crates/mun_diagnostics/src/hir/duplicate_definition_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ fn syntax_node_signature_range(
ast::StructDef::cast(syntax_node_ptr.to_node(parse.tree().syntax()))
.map_or_else(|| syntax_node_ptr.range(), |s| s.signature_range())
}
SyntaxKind::TYPE_ALIAS_DEF => {
ast::TypeAliasDef::cast(syntax_node_ptr.to_node(parse.tree().syntax()))
.map_or_else(|| syntax_node_ptr.range(), |s| s.signature_range())

Check warning on line 37 in crates/mun_diagnostics/src/hir/duplicate_definition_error.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_diagnostics/src/hir/duplicate_definition_error.rs#L36-L37

Added lines #L36 - L37 were not covered by tests
}
_ => syntax_node_ptr.range(),
}
}
Expand All @@ -58,11 +62,13 @@ fn syntax_node_identifier_range(
parse: &Parse<SourceFile>,
) -> TextRange {
match syntax_node_ptr.kind() {
SyntaxKind::FUNCTION_DEF | SyntaxKind::STRUCT_DEF => syntax_node_ptr
.to_node(parse.tree().syntax())
.children()
.find(|n| n.kind() == SyntaxKind::NAME)
.map_or_else(|| syntax_node_ptr.range(), |name| name.text_range()),
SyntaxKind::FUNCTION_DEF | SyntaxKind::STRUCT_DEF | SyntaxKind::TYPE_ALIAS_DEF => {
syntax_node_ptr
.to_node(parse.tree().syntax())
.children()
.find(|n| n.kind() == SyntaxKind::NAME)
.map_or_else(|| syntax_node_ptr.range(), |name| name.text_range())
}
_ => syntax_node_ptr.range(),
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/mun_hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,11 +826,11 @@ impl Diagnostic for PrivateTypeAlias {
}

#[derive(Debug)]

Check warning on line 828 in crates/mun_hir/src/diagnostics.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_hir/src/diagnostics.rs#L828

Added line #L828 was not covered by tests
pub struct IncoherentImpl {
pub struct ImplForForeignType {
pub impl_: InFile<AstPtr<ast::Impl>>,
}

impl Diagnostic for IncoherentImpl {
impl Diagnostic for ImplForForeignType {
fn message(&self) -> String {
String::from("cannot define inherent `impl` for foreign type")
}

Check warning on line 836 in crates/mun_hir/src/diagnostics.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_hir/src/diagnostics.rs#L834-L836

Added lines #L834 - L836 were not covered by tests
Expand Down
1 change: 1 addition & 0 deletions crates/mun_hir/src/ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub struct ModuleId {
pub local_id: LocalModuleId,
}

/// Represents an id of an item inside a item container such as a module or a `impl` block.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum ItemContainerId {
ModuleId(ModuleId),
Expand Down
18 changes: 12 additions & 6 deletions crates/mun_hir/src/item_tree/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ use std::{collections::HashMap, convert::TryInto, marker::PhantomData, sync::Arc

struct ModItems(SmallVec<[ModItem; 1]>);

struct Foo {}
impl Foo {
fn bar() {}
}

impl<T> From<T> for ModItems
where
T: Into<ModItem>,
Expand Down Expand Up @@ -77,7 +72,18 @@ impl Context {
ModItem::Function(item) => Some(&self.data.functions[item.index].name),
ModItem::Struct(item) => Some(&self.data.structs[item.index].name),
ModItem::TypeAlias(item) => Some(&self.data.type_aliases[item.index].name),
ModItem::Impl(_) | ModItem::Import(_) => None,
ModItem::Import(item) => {
let import = &self.data.imports[item.index];
if import.is_glob {
None
} else {
import
.alias
.as_ref()
.map_or_else(|| import.path.last_segment(), |alias| alias.as_name())
}
}
ModItem::Impl(_) => None,
};
if let Some(name) = name {
if let Some(first_item) = set.get(name) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/mun_hir/src/item_tree/tests.rs
expression: "print_item_tree(r#\"\n use foo::Bar;\n use baz::Bar;\n\n struct Bar {}\n \"#).unwrap()"
---
use foo::Bar;
use baz::Bar;
struct Bar {
}

18..26: the name `Bar` is defined multiple times
29..42: the name `Bar` is defined multiple times
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,5 @@ struct Baz;
type FooBar = Foo;
type FooBar = package::Foo;

80..128: the name `bar` is defined multiple times
379..406: the name `FooBar` is defined multiple times
32 changes: 30 additions & 2 deletions crates/mun_hir/src/item_tree/tests.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
use crate::{mock::MockDatabase, with_fixture::WithFixture, DefDatabase, Upcast};
use crate::{mock::MockDatabase, with_fixture::WithFixture, DefDatabase, DiagnosticSink, Upcast};
use std::fmt;

fn print_item_tree(text: &str) -> Result<String, fmt::Error> {
let (db, file_id) = MockDatabase::with_single_file(text);
let item_tree = db.item_tree(file_id);
super::pretty::print_item_tree(db.upcast(), &item_tree)
let mut result_str = super::pretty::print_item_tree(db.upcast(), &item_tree)?;
let mut sink = DiagnosticSink::new(|diag| {
result_str.push_str(&format!(
"\n{:?}: {}",
diag.highlight_range(),
diag.message()
))
});

item_tree
.diagnostics
.iter()
.for_each(|diag| diag.add_to(&db, &item_tree, &mut sink));

drop(sink);
Ok(result_str)
}

#[test]
Expand Down Expand Up @@ -62,3 +77,16 @@ fn test_impls() {
)
.unwrap());
}

#[test]
fn test_duplicate_import() {
insta::assert_snapshot!(print_item_tree(
r#"
use foo::Bar;
use baz::Bar;
struct Bar {}
"#
)
.unwrap());
}
24 changes: 13 additions & 11 deletions crates/mun_hir/src/method_resolution.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::diagnostics::{DuplicateDefinition, IncoherentImpl, InvalidSelfTyImpl};
use crate::diagnostics::{DuplicateDefinition, ImplForForeignType, InvalidSelfTyImpl};
use crate::ids::AssocItemId;
use crate::{
db::HirDatabase,
Expand All @@ -24,7 +24,7 @@ pub enum InherentImplsDiagnostics {
InvalidSelfTy(ImplId),

/// The type in the impl is not defined in the same package as the impl.
IncoherentType(ImplId),
ImplForForeignType(ImplId),

/// Duplicate definitions of an associated item
DuplicateDefinitions(AssocItemId, AssocItemId),
Expand Down Expand Up @@ -96,7 +96,7 @@ impl InherentImpls {
// Make sure the struct is defined in the same package
if s.module(db).package().id != package_defs.id {
self.diagnostics
.push(InherentImplsDiagnostics::IncoherentType(impl_id));
.push(InherentImplsDiagnostics::ImplForForeignType(impl_id));

Check warning on line 99 in crates/mun_hir/src/method_resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_hir/src/method_resolution.rs#L98-L99

Added lines #L98 - L99 were not covered by tests
}

// Add the impl to the map
Expand Down Expand Up @@ -177,13 +177,15 @@ impl InherentImplsDiagnostics {
.as_ref()
.map(AstPtr::new),
}),
InherentImplsDiagnostics::IncoherentType(impl_id) => sink.push(IncoherentImpl {
impl_: impl_id
.lookup(db.upcast())
.source(db.upcast())
.as_ref()
.map(AstPtr::new),
}),
InherentImplsDiagnostics::ImplForForeignType(impl_id) => {
sink.push(ImplForForeignType {
impl_: impl_id
.lookup(db.upcast())
.source(db.upcast())
.as_ref()
.map(AstPtr::new),
});
}

Check warning on line 188 in crates/mun_hir/src/method_resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_hir/src/method_resolution.rs#L180-L188

Added lines #L180 - L188 were not covered by tests
InherentImplsDiagnostics::DuplicateDefinitions(first, second) => {
sink.push(DuplicateDefinition {
definition: assoc_item_syntax_node_ptr(db.upcast(), second),
Expand All @@ -198,7 +200,7 @@ impl InherentImplsDiagnostics {
match self {
InherentImplsDiagnostics::LowerDiagnostic(impl_id, _)
| InherentImplsDiagnostics::InvalidSelfTy(impl_id)
| InherentImplsDiagnostics::IncoherentType(impl_id) => impl_id.module(db),
| InherentImplsDiagnostics::ImplForForeignType(impl_id) => impl_id.module(db),
InherentImplsDiagnostics::DuplicateDefinitions(_first, second) => second.module(db),

Check warning on line 204 in crates/mun_hir/src/method_resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_hir/src/method_resolution.rs#L199-L204

Added lines #L199 - L204 were not covered by tests
}
}

Check warning on line 206 in crates/mun_hir/src/method_resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mun_hir/src/method_resolution.rs#L206

Added line #L206 was not covered by tests
Expand Down
1 change: 1 addition & 0 deletions crates/mun_hir/src/package_defs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ fn use_duplicate_name() {
mod mod
+-- mod bar
| +-- ERROR: 4..20: a second item with the same name imported. Try to use an alias.
| +-- ERROR: 23..37: the name `Ok` is defined multiple times
| '-- struct Ok
'-- mod foo
'-- struct Ok
Expand Down
14 changes: 14 additions & 0 deletions crates/mun_hir/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ pub enum ImportAlias {
Alias(Name),
}

impl ImportAlias {
pub fn as_name(&self) -> Option<&Name> {
match self {
ImportAlias::Underscore => None,
ImportAlias::Alias(name) => Some(name),
}
}
}

impl Path {
/// Converts an `ast::Path` to `Path`.
pub fn from_ast(mut path: ast::Path) -> Option<Path> {
Expand Down Expand Up @@ -97,6 +106,11 @@ impl Path {
lower_use_tree(None, &tree, &mut cb);
}
}

/// Returns the last segment of the path, if any.
pub fn last_segment(&self) -> Option<&Name> {
self.segments.last()
}
}

/// Given an `ast::UseTree` and an optional prefix, call a callback function for every item that is
Expand Down
29 changes: 26 additions & 3 deletions crates/mun_syntax/src/ast/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ impl ast::NameRef {
}

impl ast::FunctionDef {
/// Returns the signature range.
///
/// ```rust, ignore
/// fn foo_bar() {
/// ^^^^^^^^^^^^___ this part
/// // ...
/// }
/// ```
pub fn signature_range(&self) -> TextRange {
let fn_kw = self
.syntax()
Expand Down Expand Up @@ -153,6 +161,15 @@ impl ast::StructDef {
pub fn kind(&self) -> StructKind {
StructKind::from_node(self)
}

/// Returns the signature range.
///
/// ```rust, ignore
/// pub(gc) struct Foo {
/// ^^^^^^^^^^___ this part
/// // ...
/// }
/// ```
pub fn signature_range(&self) -> TextRange {
let struct_kw = self
.syntax()
Expand Down Expand Up @@ -212,20 +229,26 @@ impl ast::UseTree {
}

impl ast::TypeAliasDef {
/// Returns the signature range.
///
/// ```rust, ignore
/// type FooBar = i32
/// ^^^^^^^^^^^___ this part
/// ```
pub fn signature_range(&self) -> TextRange {
let struct_kw = self
let type_kw = self
.syntax()
.children_with_tokens()
.find(|p| p.kind() == T![type])
.map(|kw| kw.text_range());
let name = self.name().map(|n| n.syntax.text_range());

let start =
struct_kw.map_or_else(|| self.syntax.text_range().start(), rowan::TextRange::start);
type_kw.map_or_else(|| self.syntax.text_range().start(), rowan::TextRange::start);

let end = name
.map(rowan::TextRange::end)
.or_else(|| struct_kw.map(rowan::TextRange::end))
.or_else(|| type_kw.map(rowan::TextRange::end))
.unwrap_or_else(|| self.syntax().text_range().end());

TextRange::new(start, end)
Expand Down

0 comments on commit 9290fd5

Please sign in to comment.