Skip to content

Commit

Permalink
Implement support for indicating the stability of items.
Browse files Browse the repository at this point in the history
There are 6 new compiler recognised attributes: deprecated, experimental,
unstable, stable, frozen, locked (these levels are taken directly from
Node's "stability index"[1]). These indicate the stability of the
item to which they are attached; e.g. `#[deprecated] fn foo() { .. }`
says that `foo` is deprecated.

This comes with 3 lints for the first 3 levels (with matching names) that
will detect the use of items marked with them (the `unstable` lint
includes items with no stability attribute). The attributes can be given
a short text note that will be displayed by the lint. An example:

    #[warn(unstable)]; // `allow` by default

    #[deprecated="use `bar`"]
    fn foo() { }

    #[stable]
    fn bar() { }

    fn baz() { }

    fn main() {
        foo(); // "warning: use of deprecated item: use `bar`"

        bar(); // all fine

        baz(); // "warning: use of unmarked item"
    }

The lints currently only check the "edges" of the AST: i.e. functions,
methods[2], structs and enum variants. Any stability attributes on modules,
enums, traits and impls are not checked.

[1]: http://nodejs.org/api/documentation.html
[2]: the method check is currently incorrect and doesn't work.
  • Loading branch information
huonw committed Sep 3, 2013
1 parent 7048e05 commit 506f69a
Show file tree
Hide file tree
Showing 8 changed files with 745 additions and 3 deletions.
58 changes: 57 additions & 1 deletion doc/rust.md
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ code_. They are defined in the same way as any other Rust function,
except that they have the `extern` modifier.

~~~
// Declares an extern fn, the ABI defaults to "C"
// Declares an extern fn, the ABI defaults to "C"
extern fn new_vec() -> ~[int] { ~[] }
// Declares an extern fn with "stdcall" ABI
Expand Down Expand Up @@ -1723,6 +1723,62 @@ Supported traits for `deriving` are:
each constituent field of the type must also implement `ToStr` and will have
`field.to_str()` invoked to build up the result.

### Stability
One can indicate the stability of an API using the following attributes:

* `deprecated`: This item should no longer be used, e.g. it has been
replaced. No guarantee of backwards-compatibility.
* `experimental`: This item was only recently introduced or is
otherwise in a state of flux. It may change significantly, or even
be removed. No guarantee of backwards-compatibility.
* `unstable`: This item is still under development, but requires more
testing to be considered stable. No guarantee of backwards-compatibility.
* `stable`: This item is considered stable, and will not change
significantly. Guarantee of backwards-compatibility.
* `frozen`: This item is very stable, and is unlikely to
change. Guarantee of backwards-compatibility.
* `locked`: This item will never change unless a serious bug is
found. Guarantee of backwards-compatibility.

These levels are directly inspired by
[Node.js' "stability index"](http://nodejs.org/api/documentation.html).

There are lints for disallowing items marked with certain levels:
`deprecated`, `experimental` and `unstable`; the first two will warn
by default. Items with not marked with a stability are considered to
be unstable for the purposes of the lint. One can give an optional
string that will be displayed when the lint flags the use of an item.

~~~ {.xfail-test}
#[warn(unstable)];
#[deprecated="replaced by `best`"]
fn bad() {
// delete everything
}
fn better() {
// delete fewer things
}
#[stable]
fn best() {
// delete nothing
}
fn main() {
bad(); // "warning: use of deprecated item: replaced by `best`"
better(); // "warning: use of unmarked item"
best(); // no warning
}
~~~

> **Note:** Currently these are only checked when applied to
> individual functions, structs, methods and enum variants, *not* to
> entire modules, traits, impls or enums themselves.
# Statements and expressions

Rust is _primarily_ an expression language. This means that most forms of
Expand Down
1 change: 1 addition & 0 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ fn encode_enum_variant_info(ecx: &EncodeContext,
encode_name(ecx, ebml_w, variant.node.name);
encode_parent_item(ebml_w, local_def(id));
encode_visibility(ebml_w, variant.node.vis);
encode_attributes(ebml_w, variant.node.attrs);
match variant.node.kind {
ast::tuple_variant_kind(ref args)
if args.len() > 0 && generics.ty_params.len() == 0 => {
Expand Down
130 changes: 129 additions & 1 deletion src/librustc/middle/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use driver::session;
use middle::ty;
use middle::pat_util;
use metadata::csearch;
use util::ppaux::{ty_to_str};

use std::cmp;
Expand All @@ -27,7 +28,7 @@ use std::u8;
use extra::smallintmap::SmallIntMap;
use syntax::ast_map;
use syntax::attr;
use syntax::attr::AttrMetaMethods;
use syntax::attr::{AttrMetaMethods, AttributeMethods};
use syntax::codemap::Span;
use syntax::codemap;
use syntax::parse::token;
Expand Down Expand Up @@ -97,6 +98,10 @@ pub enum lint {
missing_doc,
unreachable_code,

deprecated,
experimental,
unstable,

warnings,
}

Expand Down Expand Up @@ -281,6 +286,27 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
default: warn
}),

("deprecated",
LintSpec {
lint: deprecated,
desc: "detects use of #[deprecated] items",
default: warn
}),

("experimental",
LintSpec {
lint: experimental,
desc: "detects use of #[experimental] items",
default: warn
}),

("unstable",
LintSpec {
lint: unstable,
desc: "detects use of #[unstable] items (incl. items with no stability attribute)",
default: allow
}),

("warnings",
LintSpec {
lint: warnings,
Expand Down Expand Up @@ -1375,6 +1401,107 @@ fn lint_missing_doc() -> @mut OuterLint {
@mut MissingDocLintVisitor { stopping_on_items: false } as @mut OuterLint
}

/// Checks for use of items with #[deprecated], #[experimental] and
/// #[unstable] (or none of them) attributes.
struct StabilityLintVisitor { stopping_on_items: bool }

impl StabilityLintVisitor {
fn handle_def(&mut self, sp: Span, def: &ast::Def, cx: @mut Context) {
let id = ast_util::def_id_of_def(*def);

let stability = if ast_util::is_local(id) {
// this crate
match cx.tcx.items.find(&id.node) {
Some(ast_node) => {
let s = do ast_node.with_attrs |attrs| {
do attrs.map_move |a| {
attr::find_stability(a.iter().map(|a| a.meta()))
}
};
match s {
Some(s) => s,

// no possibility of having attributes
// (e.g. it's a local variable), so just
// ignore it.
None => return
}
}
_ => cx.tcx.sess.bug(fmt!("handle_def: %? not found", id))
}
} else {
// cross-crate

let mut s = None;
// run through all the attributes and take the first
// stability one.
do csearch::get_item_attrs(cx.tcx.cstore, id) |meta_items| {
if s.is_none() {
s = attr::find_stability(meta_items.move_iter())
}
}
s
};

let (lint, label) = match stability {
// no stability attributes == Unstable
None => (unstable, "unmarked"),
Some(attr::Stability { level: attr::Unstable, _ }) => (unstable, "unstable"),
Some(attr::Stability { level: attr::Experimental, _ }) => {
(experimental, "experimental")
}
Some(attr::Stability { level: attr::Deprecated, _ }) => (deprecated, "deprecated"),
_ => return
};

let msg = match stability {
Some(attr::Stability { text: Some(ref s), _ }) => {
fmt!("use of %s item: %s", label, *s)
}
_ => fmt!("use of %s item", label)
};

cx.span_lint(lint, sp, msg);
}
}

impl SubitemStoppableVisitor for StabilityLintVisitor {
fn is_running_on_items(&mut self) -> bool { !self.stopping_on_items }
}

impl Visitor<@mut Context> for StabilityLintVisitor {
fn visit_item(&mut self, i:@ast::item, e:@mut Context) {
self.OVERRIDE_visit_item(i, e);
}

fn visit_fn(&mut self, fk:&visit::fn_kind, fd:&ast::fn_decl,
b:&ast::Block, s:Span, n:ast::NodeId, e:@mut Context) {
self.OVERRIDE_visit_fn(fk, fd, b, s, n, e);
}

fn visit_expr(&mut self, ex: @ast::Expr, cx: @mut Context) {
match ex.node {
ast::ExprMethodCall(*) |
ast::ExprPath(*) |
ast::ExprStruct(*) => {
match cx.tcx.def_map.find(&ex.id) {
Some(def) => self.handle_def(ex.span, def, cx),
None => {}
}
}
_ => {}
}

visit::walk_expr(self, ex, cx)
}
}

outer_lint_boilerplate_impl!(StabilityLintVisitor)

fn lint_stability() -> @mut OuterLint {
@mut StabilityLintVisitor { stopping_on_items: false } as @mut OuterLint
}

struct LintCheckVisitor;

impl Visitor<@mut Context> for LintCheckVisitor {
Expand Down Expand Up @@ -1458,6 +1585,7 @@ pub fn check_crate(tcx: ty::ctxt, crate: @ast::Crate) {
cx.add_old_lint(lint_unused_mut());
cx.add_old_lint(lint_unnecessary_allocations());
cx.add_old_lint(lint_missing_doc());
cx.add_old_lint(lint_stability());
cx.add_lint(lint_session(cx));

// Actually perform the lint checks (iterating the ast)
Expand Down
1 change: 0 additions & 1 deletion src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4706,4 +4706,3 @@ pub fn trait_of_method(tcx: ctxt, def_id: ast::DefId)

result
}

20 changes: 20 additions & 0 deletions src/libsyntax/ast_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,26 @@ pub enum ast_node {
node_callee_scope(@Expr)
}

impl ast_node {
pub fn with_attrs<T>(&self, f: &fn(Option<&[Attribute]>) -> T) -> T {
let attrs = match *self {
node_item(i, _) => Some(i.attrs.as_slice()),
node_foreign_item(fi, _, _, _) => Some(fi.attrs.as_slice()),
node_trait_method(tm, _, _) => match *tm {
required(ref type_m) => Some(type_m.attrs.as_slice()),
provided(m) => Some(m.attrs.as_slice())
},
node_method(m, _, _) => Some(m.attrs.as_slice()),
node_variant(ref v, _, _) => Some(v.node.attrs.as_slice()),
// unit/tuple structs take the attributes straight from
// the struct definition.
node_struct_ctor(_, strct, _) => Some(strct.attrs.as_slice()),
_ => None
};
f(attrs)
}
}

pub type map = @mut HashMap<NodeId, ast_node>;

pub struct Ctx {
Expand Down
38 changes: 38 additions & 0 deletions src/libsyntax/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,44 @@ pub fn test_cfg<AM: AttrMetaMethods, It: Iterator<AM>>
no_cfgs || some_cfg_matches
}

/// Represents the #[deprecated="foo"] (etc) attributes.
pub struct Stability {
level: StabilityLevel,
text: Option<@str>
}

/// The available stability levels.
#[deriving(Eq,Ord,Clone)]
pub enum StabilityLevel {
Deprecated,
Experimental,
Unstable,
Stable,
Frozen,
Locked
}

/// Find the first stability attribute. `None` if none exists.
pub fn find_stability<AM: AttrMetaMethods, It: Iterator<AM>>(mut metas: It) -> Option<Stability> {
for m in metas {
let level = match m.name().as_slice() {
"deprecated" => Deprecated,
"experimental" => Experimental,
"unstable" => Unstable,
"stable" => Stable,
"frozen" => Frozen,
"locked" => Locked,
_ => loop // not a stability level
};

return Some(Stability {
level: level,
text: m.value_str()
});
}
None
}

pub fn require_unique_names(diagnostic: @mut span_handler,
metas: &[@MetaItem]) {
let mut set = HashSet::new();
Expand Down
Loading

5 comments on commit 506f69a

@bors
Copy link
Contributor

@bors bors commented on 506f69a Sep 3, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from brson
at huonw@506f69a

@bors
Copy link
Contributor

@bors bors commented on 506f69a Sep 3, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging huonw/rust/stability = 506f69a into auto

@bors
Copy link
Contributor

@bors bors commented on 506f69a Sep 3, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huonw/rust/stability = 506f69a merged ok, testing candidate = b4ff0bc

@bors
Copy link
Contributor

@bors bors commented on 506f69a Sep 3, 2013

@bors
Copy link
Contributor

@bors bors commented on 506f69a Sep 3, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = b4ff0bc

Please sign in to comment.