Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Make item Visibility visible in the API #294

Merged
merged 11 commits into from
Nov 14, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ The [v0.4.0 milestone] contains a list of planned changes.
[v0.4.0 milestone]: https://github.com/rust-marker/marker/milestone/4
[#278]: https://github.com/rust-marker/marker/pull/278
[#288]: https://github.com/rust-marker/marker/pull/288
[#294]: https://github.com/rust-marker/marker/pull/294
[#306]: https://github.com/rust-marker/marker/pull/306

### Added
- [#306]: The `LintPass` trait now as a new `check_crate` method.
- [#294]: Items and fields now provide information about their visibility

### Fixed
- [#306]: Rustc's driver no longer ICEs on spans from compiler generated code.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ Marker is still growing up, and that's a good thing. We can still shape the API
* Higher order types
* Attributes [#51](https://github.com/rust-marker/marker/issues/51)
* Macros [rust-marker/design#47](https://github.com/rust-marker/design/issues/47)
* Item visibility [#26](https://github.com/rust-marker/marker/issues/26)
* Item visibility [#26](https://github.com/rust-marker/marker/issues/26) (Will be added in v0.4)
* **Utility**: The API is currently lacking a lot of utility functions, to handle edge cases and make linting more pleasant.
* **Documentation**: Marker still requires a lot of documentation, in the form of doc comments and a book, which explains the basic concept and works as a guide for end-users, lint- and marker-devs alike.

Expand Down
10 changes: 3 additions & 7 deletions marker_api/src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,14 +372,10 @@ impl<'ast> ConstExpr<'ast> {

#[cfg(all(test, target_arch = "x86_64", target_pointer_width = "64"))]
mod test {
use super::*;
use expect_test::{expect, Expect};
use crate::test::assert_size_of;

#[track_caller]
fn assert_size_of<T>(expected: &Expect) {
let actual = std::mem::size_of::<T>();
expected.assert_eq(&actual.to_string());
}
use super::*;
use expect_test::expect;

#[test]
fn expr_struct_size() {
Expand Down
101 changes: 55 additions & 46 deletions marker_api/src/ast/item.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use std::{fmt::Debug, marker::PhantomData};
use std::fmt::Debug;

use crate::{
common::{HasNodeId, ItemId, SpanId},
context::with_cx,
diagnostic::EmissionNode,
ffi::FfiOption,
private::Sealed,
span::{HasSpan, Ident, Span},
CtorBlocker,
Expand Down Expand Up @@ -188,6 +190,7 @@ use impl_item_type_fn;
#[repr(C)]
#[derive(Debug)]
#[cfg_attr(feature = "driver-api", visibility::make(pub))]
#[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))]
struct CommonItemData<'ast> {
id: ItemId,
span: SpanId,
Expand Down Expand Up @@ -236,43 +239,47 @@ macro_rules! impl_item_data {

use impl_item_data;

#[cfg(feature = "driver-api")]
impl<'ast> CommonItemData<'ast> {
pub fn new(id: ItemId, span: SpanId, ident: Ident<'ast>) -> Self {
Self {
id,
span,
vis: Visibility::new(id),
ident,
}
}
}

/// This struct represents the visibility of an item.
/// The declared visibility of an item or field.
///
/// Note that this is only the syntactic visibility. The item or field might be
/// reexported with a higher visibility, or have a high default visibility.
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
///
/// ```
/// // An item without a visibility
/// fn moon() {}
///
/// // A public item
/// pub fn sun() {}
///
/// Currently, it's only a placeholder until a proper representation is implemented.
/// rust-marker/marker#26 tracks the task of implementing this. You're welcome to
/// leave any comments in that issue.
/// // An item with a restricted scope
/// pub(crate) fn star() {}
///
/// pub trait Planet {
/// // An item without a visibility. But it still has the semantic visibility
/// // of `pub` as this is inside a trait declaration.
/// fn mass();
/// }
/// ```
#[repr(C)]
#[derive(Debug)]
#[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))]
pub struct Visibility<'ast> {
_lifetime: PhantomData<&'ast ()>,
_item_id: ItemId,
#[cfg_attr(feature = "driver-api", builder(setter(into), default))]
span: FfiOption<SpanId>,
sem: crate::sem::Visibility<'ast>,
}

impl<'ast> Debug for Visibility<'ast> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Visibility {{ /* WIP: See rust-marker/marker#26 */}}")
.finish()
impl<'ast> Visibility<'ast> {
/// The [`Span`] of the visibility, if it has been declared.
pub fn span(&self) -> Option<&Span<'ast>> {
self.span.copy().map(|span| with_cx(self, |cx| cx.span(span)))
}
}

#[cfg(feature = "driver-api")]
impl<'ast> Visibility<'ast> {
pub fn new(item_id: ItemId) -> Self {
Self {
_lifetime: PhantomData,
_item_id: item_id,
}
/// This function returns the semantic representation for the [`Visibility`]
/// of this item. That visibility can be used to check if the item is public
/// or restricted to specific modules.
pub fn semantics(&self) -> &crate::sem::Visibility<'ast> {
&self.sem
}
}

Expand Down Expand Up @@ -311,26 +318,28 @@ impl<'ast> Body<'ast> {

#[cfg(all(test, target_arch = "x86_64", target_pointer_width = "64"))]
mod test {
use crate::test::assert_size_of;

use super::*;
use std::mem::size_of;
use expect_test::expect;

#[test]
fn test_item_struct_size() {
// These sizes are allowed to change, this is just a check to have a
// general overview and to prevent accidental changes
assert_eq!(56, size_of::<ModItem<'_>>(), "ModItem");
assert_eq!(48, size_of::<ExternCrateItem<'_>>(), "ExternCrateItem");
assert_eq!(64, size_of::<UseItem<'_>>(), "UseItem");
assert_eq!(80, size_of::<StaticItem<'_>>(), "StaticItem");
assert_eq!(72, size_of::<ConstItem<'_>>(), "ConstItem");
assert_eq!(144, size_of::<FnItem<'_>>(), "FnItem");
assert_eq!(112, size_of::<TyAliasItem<'_>>(), "TyAliasItem");
assert_eq!(96, size_of::<StructItem<'_>>(), "StructItem");
assert_eq!(88, size_of::<EnumItem<'_>>(), "EnumItem");
assert_eq!(88, size_of::<UnionItem<'_>>(), "UnionItem");
assert_eq!(112, size_of::<TraitItem<'_>>(), "TraitItem");
assert_eq!(144, size_of::<ImplItem<'_>>(), "ImplItem");
assert_eq!(64, size_of::<ExternBlockItem<'_>>(), "ExternBlockItem");
assert_eq!(48, size_of::<UnstableItem<'_>>(), "UnstableItem");
assert_size_of::<ModItem<'_>>(&expect!["80"]);
assert_size_of::<ExternCrateItem<'_>>(&expect!["72"]);
assert_size_of::<UseItem<'_>>(&expect!["88"]);
assert_size_of::<StaticItem<'_>>(&expect!["104"]);
assert_size_of::<ConstItem<'_>>(&expect!["96"]);
assert_size_of::<FnItem<'_>>(&expect!["168"]);
assert_size_of::<TyAliasItem<'_>>(&expect!["136"]);
assert_size_of::<StructItem<'_>>(&expect!["120"]);
assert_size_of::<EnumItem<'_>>(&expect!["112"]);
assert_size_of::<UnionItem<'_>>(&expect!["112"]);
assert_size_of::<TraitItem<'_>>(&expect!["136"]);
assert_size_of::<ImplItem<'_>>(&expect!["168"]);
assert_size_of::<ExternBlockItem<'_>>(&expect!["88"]);
assert_size_of::<UnstableItem<'_>>(&expect!["72"]);
}
}
3 changes: 2 additions & 1 deletion marker_api/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ impl std::hash::Hash for FfiStr<'_> {
/// This is an FFI safe option. In most cases it's better to pass a pointer and
/// then use `as_ref()` but this doesn't work for owned return values.
#[repr(C)]
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Default)]
pub enum FfiOption<T> {
Some(T),
#[default]
None,
}

Expand Down
10 changes: 4 additions & 6 deletions marker_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@
#![allow(clippy::missing_panics_doc)] // Temporary allow for `todo!`s
#![allow(clippy::new_without_default)] // Not very helpful as `new` is almost always cfged
#![cfg_attr(not(feature = "driver-api"), allow(dead_code))]
// FIXME(#26): this is commented out because it is the lint that we want to enable
// here makes sense only on a public items, but it triggers of private/pub(crate)
// methods today. There isn't a way to inspect the item visibility in this lint's
// impl yet. Once #26 is done and lint impl ignores private functions we may enable
// this lint.
// #![cfg_attr(marker, warn(marker::not_using_has_span_trait))]
#![cfg_attr(marker, warn(marker::marker_lints::not_using_has_span_trait))]

pub static MARKER_API_VERSION: &str = env!("CARGO_PKG_VERSION");

Expand All @@ -22,6 +17,9 @@ pub use interface::*;
mod lint;
pub use lint::*;

#[cfg(test)]
pub(crate) mod test;

pub mod ast;
pub mod common;
pub mod context;
Expand Down
2 changes: 2 additions & 0 deletions marker_api/src/sem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

mod common;
mod generic;
mod item;
mod ty;

pub use common::*;
pub use generic::*;
pub use item::*;
pub use ty::*;
153 changes: 153 additions & 0 deletions marker_api/src/sem/item.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
use std::marker::PhantomData;

use crate::common::ItemId;

/// The declared visibility of an item or field.
///
/// ```
/// // An item without a visibility
/// fn moon() {}
///
/// // A public item
/// pub fn sun() {}
///
/// // An item with a restricted scope
/// pub(crate) fn star() {}
///
/// pub trait Planet {
/// // An item without a visibility. But it still has the semantic visibility
/// // of `pub` as this is inside a trait declaration.
/// fn mass();
/// }
/// ```
#[repr(C)]
#[derive(Debug)]
#[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))]
pub struct Visibility<'ast> {
#[cfg_attr(feature = "driver-api", builder(setter(skip), default))]
_lifetime: PhantomData<&'ast ()>,
kind: VisibilityKind,
}

impl<'ast> Visibility<'ast> {
/// Returns `true` if the item is public, meaning that it can be visible outside
/// the crate.
///
/// ```
/// // This returns `true`
/// pub fn unicorn() {}
///
/// // This returns `false`, since the visibility is restricted to a specified path.
/// pub(crate) fn giraffe() {}
///
/// // This returns `false`, since the visibility is not defined
/// fn dragon() {}
///
/// pub trait MythicalCreature {
/// // This returns `true`, since the default visibility for trait items is public
/// fn name() -> &'static str;
/// }
/// ```
pub fn is_pub(&self) -> bool {
self.scope().is_none()
}

/// Returns `true` if the item is visible in the entire crate. This is
/// the case for items declared as `pub(crate)`. Items declared in the root
/// module of the crate or specify the path of the root module are also
/// scoped to the entire crate.
///
/// ```
/// // lib.rs
///
/// mod example_1 {
/// // Returns `false` since it's only visible in `crate::example_1`
/// fn foo() {}
///
/// // Returns `false` since it's only visible in `crate::example_1`
/// pub(in crate::example_1) fn bar() {}
///
/// // Returns `true` as the visibility is restricted to the root of the crate.
/// pub(crate) fn baz() {}
///
/// // Returns `true` as the visibility is restricted to `super` which is the
/// // root of the crate.
/// pub(super) fn boo() {}
/// }
///
/// // Returns `true` since this item is at the root of the crate and the default
/// // visibility is therefore `pub(crate)`
/// fn example_in_root() {}
///
/// # fn main() {}
/// ```
pub fn is_crate_scoped(&self) -> bool {
matches!(self.kind, VisibilityKind::Crate(_) | VisibilityKind::DefaultCrate(_))
}

/// Returns `true` if a visibility is the default visibility, meaning that it wasn't
/// declared.
pub fn is_default(&self) -> bool {
matches!(
self.kind,
VisibilityKind::DefaultPub | VisibilityKind::DefaultCrate(_) | VisibilityKind::Default(_)
)
}

/// Returns the [`ItemId`] of the module that this item or field is visible in.
/// It will return `None`, if the item is public, as the visibility extends past
/// the root module of the crate.
///
/// This function also works for items which have the default visibility, of the
/// module they are declared in.
///
/// ```
/// // lib.rs
///
/// mod scope {
/// // Returns `None` since this is even visible outside the current crate
/// pub fn turtle() {}
///
/// // Returns the `ItemId` of the root module of the crate
/// pub(crate) fn shark() {}
///
/// // Returns the `ItemId` of the module it is declared in, in this case `crate::scope`
/// fn dolphin() {}
/// }
/// ```
pub fn scope(&self) -> Option<ItemId> {
match self.kind {
VisibilityKind::Path(id)
| VisibilityKind::Crate(id)
| VisibilityKind::DefaultCrate(id)
| VisibilityKind::Default(id) => Some(id),
_ => None,
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
}
}

// FIXME(xFrednet): Implement functions to check if an item is visible from a
// given `ItemId`. This can be done once rust-marker/marker#242 is implemented.
}

#[derive(Debug)]
#[allow(clippy::exhaustive_enums)]
#[cfg_attr(feature = "driver-api", visibility::make(pub))]
enum VisibilityKind {
/// The item is declared as `pub` without any restrictions
Public,
/// For items which are `pub` by default, like trait functions or enum variants
DefaultPub,
/// The visibility is restricted to the root module of the crate. The [`ItemId`]
/// identifies the root module.
Crate(ItemId),
/// The items doesn't have a declared visibility. The default is visible in the
/// entire crate. The [`ItemId`] identifies the root module.
DefaultCrate(ItemId),
/// The visibility is restricted to a specific module using `pub(<path>)`.
/// The module, targeted by the path is identified by the [`ItemId`].
/// The `pub(crate)` has it's own variant in this struct.
Path(ItemId),
/// The items doesn't have a declared visibility. The default is restricted to
/// a module, identified by the stored [`ItemId`]
Default(ItemId),
}
7 changes: 7 additions & 0 deletions marker_api/src/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
use expect_test::Expect;

#[track_caller]
pub(crate) fn assert_size_of<T>(expected: &Expect) {
let actual = std::mem::size_of::<T>();
expected.assert_eq(&actual.to_string());
}
Loading