Skip to content

Commit

Permalink
Feat: Added private type leak validation to structs
Browse files Browse the repository at this point in the history
Also exposed struct field visibility in the HIR and created a struct validation module
  • Loading branch information
mkhan45 committed Aug 9, 2021
1 parent 69594b0 commit f8e20c1
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 39 deletions.
2 changes: 1 addition & 1 deletion crates/mun_hir/src/code_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod function;
mod module;
mod package;
pub(crate) mod src;
mod r#struct;
pub mod r#struct;
mod type_alias;

use crate::{expr::BodySourceMap, HirDatabase, Name};
Expand Down
9 changes: 8 additions & 1 deletion crates/mun_hir/src/code_model/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
};
use mun_syntax::{
ast,
ast::{NameOwner, TypeAscriptionOwner},
ast::{NameOwner, TypeAscriptionOwner, VisibilityOwner},
};
use std::{fmt, sync::Arc};

Expand All @@ -19,6 +19,8 @@ use crate::visibility::RawVisibility;
pub use ast::StructMemoryKind;
use std::iter::once;

pub mod validator;

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct Struct {
pub(crate) id: StructId,
Expand Down Expand Up @@ -125,6 +127,8 @@ impl Struct {
let data = self.data(db.upcast());
let lower = self.lower(db);
lower.add_diagnostics(db, self.file_id(db), data.type_ref_source_map(), sink);
let validator = validator::StructValidator::new(self, db, self.file_id(db));
validator.validate_privacy(sink);
}
}

Expand All @@ -144,6 +148,7 @@ impl Struct {
pub struct FieldData {
pub name: Name,
pub type_ref: LocalTypeRefId,
pub visibility: RawVisibility,
}

/// A struct's fields' data (record, tuple, or unit struct)
Expand Down Expand Up @@ -198,6 +203,7 @@ impl StructData {
.map(|fd| FieldData {
name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing),
type_ref: type_ref_builder.alloc_from_node_opt(fd.ascribed_type().as_ref()),
visibility: RawVisibility::from_ast(fd.visibility()),
})
.collect();
(fields, StructKind::Record)
Expand All @@ -209,6 +215,7 @@ impl StructData {
.map(|(index, fd)| FieldData {
name: Name::new_tuple_field(index),
type_ref: type_ref_builder.alloc_from_node_opt(fd.type_ref().as_ref()),
visibility: RawVisibility::from_ast(fd.visibility()),
})
.collect();
(fields, StructKind::Tuple)
Expand Down
65 changes: 65 additions & 0 deletions crates/mun_hir/src/code_model/struct/validator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use super::Struct;
use crate::diagnostics::ExportedPrivate;
use crate::resolve::HasResolver;
use crate::DiagnosticSink;
use crate::{HasVisibility, Ty, Visibility};

use crate::FileId;
use crate::HirDatabase;

use crate::visibility::RawVisibility;

#[cfg(test)]
mod tests;

pub struct StructValidator<'a> {
strukt: Struct,
db: &'a dyn HirDatabase,
file_id: FileId,
}

impl<'a> StructValidator<'a> {
pub fn new(strukt: Struct, db: &'a dyn HirDatabase, file_id: FileId) -> Self {
StructValidator {
strukt,
db,
file_id,
}
}

pub fn validate_privacy(&self, sink: &mut DiagnosticSink) {
let resolver = self.strukt.id.resolver(self.db.upcast());
let struct_data = self.strukt.data(self.db.upcast());

let public_fields = struct_data
.fields
.iter()
.filter(|(_, field_data)| field_data.visibility == RawVisibility::Public);

let field_types = public_fields.map(|(_, field_data)| {
let type_ref = field_data.type_ref;
let ty = Ty::from_hir(self.db, &resolver, &struct_data.type_ref_map(), type_ref).ty;
(ty, type_ref)
});

let struct_visibility = self.strukt.visibility(self.db);
let type_is_allowed = |ty: &Ty| match struct_visibility {
Visibility::Module(module_id) => {
ty.visibility(self.db).is_visible_from(self.db, module_id)
}
Visibility::Public => ty.visibility(self.db).is_externally_visible(),
};

field_types
.filter(|(ty, _)| !type_is_allowed(&ty))
.for_each(|(_, type_ref)| {
sink.push(ExportedPrivate {
file: self.file_id,
type_ref: struct_data
.type_ref_source_map()
.type_ref_syntax(type_ref)
.unwrap(),
})
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/mun_hir/src/code_model/struct/validator/tests.rs
expression: "struct Foo(usize);\npub struct Bar(usize);\n\n// valid, bar is public\npub struct Baz {\n foo: Foo,\n pub bar: Bar,\n}\n\n// invalid, Foo is private\npub struct FooBar {\n pub foo: Foo,\n pub bar: Bar,\n}\n\n// valid, FooBaz is private\nstruct FooBaz {\n pub foo: Foo,\n pub bar: Bar,\n}\n\npub(crate) struct BarBaz;\n\n// invalid, exporting pub(crate) to pub\npub struct FooBarBaz {\n pub foo: Foo,\n pub bar: Bar,\n}"

---
179..182: can't leak private type
391..394: can't leak private type

47 changes: 47 additions & 0 deletions crates/mun_hir/src/code_model/struct/validator/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#[cfg(test)]
use crate::utils::tests::*;

#[test]
fn test_private_leak_struct_fields() {
diagnostics_snapshot(
r#"
struct Foo(usize);
pub struct Bar(usize);
// valid, bar is public
pub struct Baz {
foo: Foo,
pub bar: Bar,
}
// invalid, Foo is private
pub struct FooBar {
pub foo: Foo,
pub bar: Bar,
}
// valid, FooBaz is private
struct FooBaz {
pub foo: Foo,
pub bar: Bar,
}
pub(crate) struct BarBaz;
// invalid, exporting pub(crate) to pub
pub struct FooBarBaz {
pub foo: Foo,
pub bar: Bar,
}
"#,
)
}

// this function needs to be declared in each file separately
// since insta's AutoName creates files in the directory from which
// the macro is called.
fn diagnostics_snapshot(text: &str) {
let text = text.trim().replace("\n ", "\n");
insta::assert_snapshot!(insta::_macro_support::AutoName, diagnostics(&text), &text);
}
42 changes: 5 additions & 37 deletions crates/mun_hir/src/expr/validator/tests.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
use crate::{
diagnostics::DiagnosticSink,
expr::validator::{ExprValidator, TypeAliasValidator},
mock::MockDatabase,
with_fixture::WithFixture,
ModuleDef, Package,
};
use std::fmt::Write;
#[cfg(test)]
use crate::utils::tests::*;

#[test]
fn test_uninitialized_access() {
Expand Down Expand Up @@ -161,35 +155,9 @@ fn test_private_leak_alias() {
)
}

fn diagnostics(content: &str) -> String {
let (db, _file_id) = MockDatabase::with_single_file(content);

let mut diags = String::new();

let mut diag_sink = DiagnosticSink::new(|diag| {
write!(diags, "{:?}: {}\n", diag.highlight_range(), diag.message()).unwrap();
});

for item in Package::all(&db)
.iter()
.flat_map(|pkg| pkg.modules(&db))
.flat_map(|module| module.declarations(&db))
{
match item {
ModuleDef::Function(item) => {
ExprValidator::new(item, &db).validate_body(&mut diag_sink);
}
ModuleDef::TypeAlias(item) => {
TypeAliasValidator::new(item, &db).validate_target_type_existence(&mut diag_sink);
}
_ => {}
}
}

drop(diag_sink);
diags
}

// this function needs to be declared in each file separately
// since insta's AutoName creates files in the directory from which
// the macro is called.
fn diagnostics_snapshot(text: &str) {
let text = text.trim().replace("\n ", "\n");
insta::assert_snapshot!(insta::_macro_support::AutoName, diagnostics(&text), &text);
Expand Down
47 changes: 47 additions & 0 deletions crates/mun_hir/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,50 @@ pub(crate) fn make_mut_slice<T: Clone>(a: &mut Arc<[T]>) -> &mut [T] {
}
Arc::get_mut(a).unwrap()
}

#[cfg(test)]
pub mod tests {
use std::fmt::Write;

use crate::{
code_model::r#struct::validator::StructValidator,
diagnostics::DiagnosticSink,
expr::validator::{ExprValidator, TypeAliasValidator},
mock::MockDatabase,
with_fixture::WithFixture,
FileId, ModuleDef, Package,
};

pub fn diagnostics(content: &str) -> String {
let (db, _file_id) = MockDatabase::with_single_file(content);

let mut diags = String::new();

let mut diag_sink = DiagnosticSink::new(|diag| {
writeln!(diags, "{:?}: {}", diag.highlight_range(), diag.message()).unwrap();
});

for item in Package::all(&db)
.iter()
.flat_map(|pkg| pkg.modules(&db))
.flat_map(|module| module.declarations(&db))
{
match item {
ModuleDef::Function(item) => {
ExprValidator::new(item, &db).validate_body(&mut diag_sink);
}
ModuleDef::TypeAlias(item) => {
TypeAliasValidator::new(item, &db)
.validate_target_type_existence(&mut diag_sink);
}
ModuleDef::Struct(item) => {
StructValidator::new(item, &db, FileId(0)).validate_privacy(&mut diag_sink);
}
_ => {}
}
}

drop(diag_sink);
diags
}
}

0 comments on commit f8e20c1

Please sign in to comment.