Improve lints: gather info on per-instance basis.

This commit is contained in:
Sergio Benitez 2017-01-31 02:01:30 -08:00
parent 4eaf9ba9c5
commit c1697509ba
8 changed files with 350 additions and 113 deletions

View File

@ -100,6 +100,7 @@
#[macro_use] extern crate rustc;
extern crate syntax;
extern crate syntax_ext;
extern crate syntax_pos;
extern crate rustc_plugin;
extern crate rocket;
@ -177,5 +178,5 @@ pub fn plugin_registrar(reg: &mut Registry) {
// "options" => options_decorator
);
register_lints!(reg, ManagedStateLint);
register_lints!(reg, RocketLint);
}

View File

@ -1,5 +1,3 @@
extern crate syntax_pos;
mod utils;
use self::utils::*;
@ -7,71 +5,185 @@ use self::utils::*;
use ::{ROUTE_ATTR, ROUTE_INFO_ATTR};
use std::mem::transmute;
use std::collections::{HashSet, HashMap};
use std::collections::HashMap;
use rustc::lint::Level;
use rustc::lint::{LateContext, LintContext, LintPass, LateLintPass, LintArray};
use rustc::hir::{Item, Expr, Crate};
use rustc::lint::{Level, LateContext, LintContext, LintPass, LateLintPass, LintArray};
use rustc::hir::{Item, Expr, Crate, Decl, FnDecl, Body, QPath, PatKind};
use rustc::hir::def::Def;
use rustc::hir::def_id::DefId;
use rustc::ty::Ty;
use rustc::hir::intravisit::{FnKind};
use rustc::hir::{FnDecl, Body};
use rustc::hir::Ty_::TyPath;
use self::syntax_pos::Span;
use rustc::hir::Ty_::*;
use rustc::hir::Decl_::*;
use rustc::hir::Expr_::*;
use syntax_pos::Span;
use syntax::symbol::Symbol as Name;
use syntax::ast::NodeId;
const STATE_TYPE: &'static [&'static str] = &["rocket", "request", "state", "State"];
// Information about a specific Rocket instance.
#[derive(Debug, Default)]
pub struct ManagedStateLint {
struct InstanceInfo {
// Mapping from mounted struct info to the span of the mounted call.
mounted: HashMap<DefId, Span>,
// Mapping from managed types to the span of the manage call.
managed: HashMap<Ty<'static>, Span>,
}
/// A `Receiver` captures the "receiver" of a Rocket instance method call. A
/// Receiver can be an existing instance of Rocket or a call to an Rocket
/// initialization function.
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
enum Receiver {
Instance(DefId, Span),
Call(NodeId, Span),
}
impl Receiver {
/// Returns the span associated with the receiver.
pub fn span(&self) -> Span {
match *self {
Receiver::Instance(_, sp) | Receiver::Call(_, sp) => sp
}
}
}
#[derive(Debug, Default)]
pub struct RocketLint {
// All of the types that were requested as managed state.
// (fn_name, fn_span, info_struct_def_id, req_type, req_param_span)
requested: Vec<(Name, Span, DefId, Ty<'static>, Span)>,
// The DefId of all of the route infos for the mounted routes.
mounted: HashSet<DefId>,
// The names of all of the routes that were declared.
// Mapping from a `Rocket` instance initialization call span (an ignite or
// custom call) to the collected info about that instance.
instances: HashMap<Option<Receiver>, InstanceInfo>,
// Map of all route info structure names found in the program to its defid.
// This is used to map a declared route to its info structure defid.
info_structs: HashMap<Name, DefId>,
// The name, span, and info DefId for all declared route functions.
// The name, span, and info DefId for all route functions found. The DefId
// is obtained by indexing into info_structs with the name found in the
// attribute that Rocket generates.
declared: Vec<(Name, Span, DefId)>,
// The expressions that were passed into a `.manage` call.
managed: Vec<(Ty<'static>, Span)>,
// Span for rocket::ignite() or rocket::custom().
start_call: Vec<Span>,
// Mapping from known named Rocket instances to initial receiver. This is
// used to do a sort-of flow-based analysis. We track variable declarations
// and link calls to Rocket methods to the (as best as we can tell) initial
// call to generate that Rocket instance. We use this to group calls to
// `manage` and `mount` to give more accurate warnings.
instance_vars: HashMap<DefId, Receiver>,
}
declare_lint!(UNMOUNTED_ROUTE, Warn, "Warn on routes that are unmounted.");
declare_lint!(UNMANAGED_STATE, Warn, "Warn on declared use on unmanaged state.");
impl<'tcx> LintPass for ManagedStateLint {
declare_lint!(UNMANAGED_STATE, Warn, "Warn on declared use of unmanaged state.");
impl<'tcx> LintPass for RocketLint {
fn get_lints(&self) -> LintArray {
lint_array!(UNMANAGED_STATE, UNMOUNTED_ROUTE)
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManagedStateLint {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let Some(args) = rocket_method_call("manage", cx, expr) {
let expr = &args[0];
if let Some(ty) = cx.tables.expr_ty_opt(expr) {
let casted = unsafe { transmute(ty) };
self.managed.push((casted, expr.span));
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RocketLint {
// This fills out the `instance_vars` table by tracking calls to
// function/methods that create Rocket instances. If the call is a method
// call with a receiver that we know is a Rocket instance, then we know it's
// been moved, and we track that move by linking all definition to the same
// receiver.
fn check_decl(&mut self, cx: &LateContext<'a, 'tcx>, decl: &'tcx Decl) {
// We only care about local declarations...everything else seems very
// unlikely. This is imperfect, after all.
if let DeclLocal(ref local) = decl.node {
// Retrieve the def_id for the new binding.
let new_def_id = match local.pat.node {
PatKind::Binding(_, def_id, ..) => def_id ,
_ => return
};
if let Some(args) = rocket_method_call("mount", cx, expr) {
for def_id in extract_mount_fn_def_ids(cx, &args[1]) {
self.mounted.insert(def_id);
}
}
// `init` is the RHS of the declaration.
if let Some(ref init) = local.init {
// We only care about declarations that result in Rocket insts.
if !returns_rocket_instance(cx, init) {
return;
}
if is_rocket_start_call(cx, expr) {
self.start_call.push(expr.span);
let (expr, span) = match find_initial_receiver(cx, init) {
Some(expr) => (expr, expr.span),
None => return
};
// If the receiver is a path, check if this path was declared
// before by another binding and use that binding's receiver as
// this binding's receiver, essentially taking us back in time.
// If we don't know about it, just insert a new receiver.
if let ExprPath(QPath::Resolved(_, ref path)) = expr.node {
if let Some(old_def_id) = path.def.def_id_opt() {
if let Some(&prev) = self.instance_vars.get(&old_def_id) {
self.instance_vars.insert(new_def_id, prev);
} else {
let recvr = Receiver::Instance(old_def_id, span);
self.instance_vars.insert(new_def_id, recvr);
}
}
}
// We use a call as a base case. Maybe it's a brand new Rocket
// instance, maybe it's a function returning a Rocket instance.
// Who knows. This is where imperfection comes in. We're just
// going to assume that calls to `mount` and `manage` are
// grouped with their originating call.
if let ExprCall(ref expr, ..) = expr.node {
let recvr = Receiver::Call(expr.id, span);
self.instance_vars.insert(new_def_id, recvr);
}
}
}
}
// Here, we collect all of the calls to `manage` and `mount` by instance,
// where the instance is determined by the receiver of the call. We look up
// the receiver in the type table we've constructed. If it's there, we use
// it, if not, we use the call as the receiver.
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
/// Fetches the top-level `Receiver` instance given that a method call
/// was made to the receiver `rexpr`. Top-level here means "the
/// original". We search the `instance_vars` table to retrieve it.
let instance_for = |lint: &mut RocketLint, rexpr: &Expr| -> Option<Receiver> {
match rexpr.node {
ExprPath(QPath::Resolved(_, ref p)) => {
p.def.def_id_opt()
.and_then(|id| lint.instance_vars.get(&id))
.map(|recvr| recvr.clone())
}
ExprCall(ref c, ..) => Some(Receiver::Call(c.id, rexpr.span)),
_ => unreachable!()
}
};
if let Some((recvr, args)) = rocket_method_call("manage", cx, expr) {
let managed_val = &args[0];
let instance = recvr.and_then(|r| instance_for(self, r));
if let Some(managed_ty) = cx.tables.expr_ty_opt(managed_val) {
self.instances.entry(instance)
.or_insert_with(|| InstanceInfo::default())
.managed
.insert(unsafe { transmute(managed_ty) }, managed_val.span);
}
}
if let Some((recvr, args)) = rocket_method_call("mount", cx, expr) {
let instance = recvr.and_then(|r| instance_for(self, r));
for def_id in extract_mount_fn_def_ids(cx, &args[1]) {
self.instances.entry(instance)
.or_insert_with(|| InstanceInfo::default())
.mounted
.insert(def_id, expr.span);
}
}
}
// We collect all of the names and defids for the info structures that
// Rocket has generated. We do this by simply looking at the attribute,
// which Rocket's codegen was kind enough to generate.
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
// Return early if this is not a route info structure.
if !item.attrs.iter().any(|attr| attr.check_name(ROUTE_INFO_ATTR)) {
@ -83,14 +195,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManagedStateLint {
}
}
/// We do two things here: 1) we find all of the `State` request guards a
/// user wants, and 2) we find all of the routes declared by the user. We
/// determine that a function is a route by looking for the attribute that
/// Rocket declared. We tie the route to the info structure, obtained from
/// the `check_item` call, so that we can determine if the route was mounted
/// or not. The tie is done by looking at the name of the info structure in
/// the attribute that Rocket generated and then looking up the structure in
/// the `info_structs` map. The structure _must_ be there since Rocket
/// always generates the structure before the route.
fn check_fn(&mut self,
cx: &LateContext<'a, 'tcx>,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl,
_: &'tcx Body,
fn_sp: Span,
fn_id: NodeId)
{
fn_id: NodeId) {
// Get the name of the function, if any.
let fn_name = match kind {
FnKind::ItemFn(name, ..) => name,
@ -100,15 +220,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManagedStateLint {
// Figure out if this is a route function by trying to find the
// `ROUTE_ATTR` attribute and extracing the info struct's name from it.
let attr_value = kind.attrs().iter().filter_map(|attr| {
if !attr.check_name(ROUTE_ATTR) {
None
} else {
attr.value.meta_item_list().and_then(|list| list[0].name())
match attr.check_name(ROUTE_ATTR) {
false => None,
true => attr.value.meta_item_list().and_then(|list| list[0].name())
}
}).next();
// Try to get the DEF_ID using the info struct's name. Return early if
// anything goes awry.
// Try to get the DEF_ID using the info struct's name from the
// `info_structs` map. Return early if anything goes awry.
let def_id = match attr_value {
Some(val) if self.info_structs.contains_key(&val) => {
self.info_structs.get(&val).unwrap()
@ -137,13 +256,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManagedStateLint {
None => continue
};
if !match_def_path(cx.tcx, def_id, STATE_TYPE) {
continue;
}
if let Some(inner_type) = input_ty.walk_shallow().next() {
let casted = unsafe { transmute(inner_type) };
tys.push(casted);
if match_def_path(cx.tcx, def_id, STATE_TYPE) {
if let Some(inner_type) = input_ty.walk_shallow().next() {
tys.push(unsafe { transmute(inner_type) });
}
}
}
}
@ -163,8 +279,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManagedStateLint {
// Sanity check: we should have as many spans as types.
if tys.len() != spans.len() {
panic!("Internal lint error: mismatched type/spans: {}/{}",
tys.len(), spans.len());
panic!("Lint error: unequal ty/span ({}/{})", tys.len(), spans.len());
}
// Insert the information we've collected.
@ -174,47 +289,44 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManagedStateLint {
}
fn check_crate_post(&mut self, cx: &LateContext<'a, 'tcx>, _: &'tcx Crate) {
// Record the start function span if we found one, and only one.
// TODO: Try to find the _right_ one using some heuristics.
let start_call_sp = match self.start_call.len() == 1 {
true => Some(self.start_call[0]),
false => None
};
// Iterate through all the instances, emitting warnings.
for (instance, info) in self.instances.iter() {
self.unmounted_warnings(cx, *instance, info);
self.unmanaged_warnings(cx, *instance, info);
}
}
}
impl RocketLint {
fn unmounted_warnings(&self, cx: &LateContext,
rcvr: Option<Receiver>,
info: &InstanceInfo) {
// Emit a warning for all unmounted, declared routes.
for &(route_name, fn_sp, info_def_id) in self.declared.iter() {
if !self.mounted.contains(&info_def_id) {
let msg = format!("the '{}' route is not mounted", route_name);
let mut b = cx.struct_span_lint(UNMOUNTED_ROUTE, fn_sp, &msg);
b.note("Rocket will not dispatch requests to unmounted routes");
if let Some(start_sp) = start_call_sp {
b.span_help(start_sp, "maybe missing a call to 'mount' here?");
}
b.emit();
if !info.mounted.contains_key(&info_def_id) {
let help_span = rcvr.map(|r| r.span());
msg_and_help(cx, UNMOUNTED_ROUTE, fn_sp,
&format!("the '{}' route is not mounted", route_name),
"Rocket will not dispatch requests to unmounted routes.",
help_span, "maybe add a call to `mount` here?");
}
}
}
let managed_types: HashSet<Ty> = self.managed.iter()
.map(|&(ty, _)| ty)
.collect();
fn unmanaged_warnings(&self,
cx: &LateContext,
rcvr: Option<Receiver>,
info: &InstanceInfo) {
for &(_, _, info_def_id, ty, sp) in self.requested.iter() {
// Don't warn on unmounted routes.
if !self.mounted.contains(&info_def_id) {
continue
}
if !info.mounted.contains_key(&info_def_id) { continue }
if !managed_types.contains(&ty) {
let m = format!("'{}' is not currently being managed by Rocket", ty);
let mut b = cx.struct_span_lint(UNMANAGED_STATE, sp, &m);
b.note("this 'State' request guard will always fail");
if let Some(start_sp) = start_call_sp {
let msg = format!("maybe missing a call to 'manage' here?");
b.span_help(start_sp, &msg);
}
b.emit()
if !info.managed.contains_key(&ty) {
let help_span = rcvr.map(|r| r.span());
msg_and_help(cx, UNMANAGED_STATE, sp,
&format!("'{}' is not currently being managed by Rocket", ty),
"this 'State' request guard will always fail",
help_span, "maybe add a call to `manage` here?");
}
}
}

View File

@ -1,31 +1,27 @@
use rustc::ty;
use rustc::hir::def_id::DefId;
use rustc::lint::LateContext;
use rustc::lint::{LintContext, Lint, LateContext};
use rustc::hir::Expr_::*;
use rustc::hir::Expr;
use rustc::hir::def::Def;
use syntax::symbol;
use syntax_pos::Span;
const ROCKET_TYPE: &'static [&'static str] = &["rocket", "rocket", "Rocket"];
const ROCKET_IGNITE_FN: &'static [&'static str] = &["rocket", "ignite"];
const ROCKET_IGNITE_STATIC: &'static [&'static str]
= &["rocket", "rocket", "Rocket", "ignite"];
const ROCKET_IGNITE_STATIC: &'static [&'static str] = &["rocket", "rocket",
"Rocket", "ignite"];
const ROCKET_CUSTOM_FN: &'static [&'static str] = &["rocket", "custom"];
const ROCKET_CUSTOM_STATIC: &'static [&'static str]
= &["rocket", "rocket", "Rocket", "custom"];
const ROCKET_CUSTOM_STATIC: &'static [&'static str] = &["rocket", "rocket",
"Rocket", "custom"];
const ABSOLUTE: &'static ty::item_path::RootMode = &ty::item_path::RootMode::Absolute;
const ABSOLUTE: &'static ty::item_path::RootMode =
&ty::item_path::RootMode::Absolute;
/// Check if a `DefId`'s path matches the given absolute type path usage.
///
/// # Examples
/// ```rust,ignore
/// match_def_path(cx.tcx, id, &["core", "option", "Option"])
/// ```
///
/// See also the `paths` module.
pub fn match_def_path(tcx: ty::TyCtxt, def_id: DefId, path: &[&str]) -> bool {
struct AbsolutePathBuffer {
names: Vec<symbol::InternedString>,
@ -64,12 +60,26 @@ pub fn is_impl_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool {
}
}
pub fn rocket_method_call<'e>(
method: &str, cx: &LateContext, expr: &'e Expr
) -> Option<&'e [Expr]> {
pub fn find_initial_receiver<'e>(cx: &LateContext,
expr: &'e Expr)
-> Option<&'e Expr> {
match expr.node {
ExprMethodCall(_, _, ref args) => find_initial_receiver(cx, &args[0]),
ExprCall(..) if is_rocket_start_call(cx, expr) => Some(expr),
ExprCall(ref call, _) => find_initial_receiver(cx, call),
ExprPath(_) => Some(expr),
_ => None,
}
}
pub fn rocket_method_call<'e>(method: &str,
cx: &LateContext,
expr: &'e Expr)
-> (Option<(Option<&'e Expr>, &'e [Expr])>) {
if let ExprMethodCall(ref name, _, ref exprs) = expr.node {
if &*name.node.as_str() == method && is_impl_method(cx, expr, ROCKET_TYPE) {
return Some(&exprs[1..]);
let receiver = find_initial_receiver(cx, &exprs[0]);
return Some((receiver, &exprs[1..]));
}
}
@ -81,13 +91,13 @@ pub fn is_rocket_start_call(cx: &LateContext, expr: &Expr) -> bool {
if let ExprPath(ref qpath) = expr.node {
let def_id = cx.tables.qpath_def(qpath, expr.id).def_id();
if match_def_path(cx.tcx, def_id, ROCKET_IGNITE_FN) {
return true
return true;
} else if match_def_path(cx.tcx, def_id, ROCKET_IGNITE_STATIC) {
return true
return true;
} else if match_def_path(cx.tcx, def_id, ROCKET_CUSTOM_FN) {
return true
return true;
} else if is_impl_method(cx, expr, ROCKET_CUSTOM_STATIC) {
return true
return true;
}
}
}
@ -125,3 +135,62 @@ pub fn extract_mount_fn_def_ids(cx: &LateContext, expr: &Expr) -> Vec<DefId> {
output
}
pub fn returns_rocket_instance(cx: &LateContext, expr: &Expr) -> bool {
if let Some(ref ty) = cx.tables.expr_ty_opt(expr) {
if let Some(def_id) = ty.ty_to_def_id() {
if match_def_path(cx.tcx, def_id, ROCKET_TYPE) {
return true;
}
}
}
false
}
pub trait DefExt {
fn def_id_opt(&self) -> Option<DefId>;
}
impl DefExt for Def {
fn def_id_opt(&self) -> Option<DefId> {
match *self {
Def::Fn(id) |
Def::Mod(id) |
Def::Static(id, _) |
Def::Variant(id) |
Def::VariantCtor(id, ..) |
Def::Enum(id) |
Def::TyAlias(id) |
Def::AssociatedTy(id) |
Def::TyParam(id) |
Def::Struct(id) |
Def::StructCtor(id, ..) |
Def::Union(id) |
Def::Trait(id) |
Def::Method(id) |
Def::Const(id) |
Def::AssociatedConst(id) |
Def::Local(id) |
Def::Upvar(id, ..) |
Def::Macro(id) => Some(id),
Def::Label(..) | Def::PrimTy(..) | Def::SelfTy(..) | Def::Err => None,
}
}
}
pub fn msg_and_help<'a, T: LintContext<'a>>(cx: &T,
lint: &'static Lint,
msg_sp: Span,
msg: &str,
note: &str,
help_sp: Option<Span>,
help: &str) {
let mut b = cx.struct_span_lint(lint, msg_sp, msg);
if let Some(span) = help_sp {
b.span_help(span, help);
}
b.note(note);
b.emit();
}

View File

@ -22,11 +22,13 @@ mod external {
#[get("/state/extern")]
fn unmanaged_unmounted(_c: ::State<u8>) { }
//~^ WARN is not mounted
//~^^ WARN is not mounted
}
#[get("/state/bad")]
fn unmanaged(_b: State<MySecondType>) { }
//~^ ERROR not currently being managed
//~^^ ERROR not currently being managed
#[get("/state/ok")]
fn managed(_a: State<u32>) { }
@ -37,19 +39,32 @@ fn managed_two(_b: State<MyType>) { }
#[get("/state/ok")]
fn unmounted_doesnt_error(_a: State<i8>) { }
//~^ WARN is not mounted
//~^^ WARN is not mounted
#[get("/ignored")]
#[allow(unmanaged_state)]
fn ignored(_b: State<u16>) { }
//~^ WARN is not mounted
#[get("/unmounted/ignored")]
#[allow(unmounted_route)]
fn unmounted_ignored() { }
#[get("/mounted/nonce")]
fn mounted_only_once() { }
//~^ WARN is not mounted
fn main() {
rocket::ignite()
.mount("/", routes![managed, unmanaged, external::unmanaged])
.mount("/", routes![managed_two, ignored])
.mount("/", routes![managed_two, ignored, mounted_only_once])
.manage(MyType)
.manage(100u32);
rocket::ignite()
.mount("/", routes![managed, unmanaged, external::unmanaged])
.mount("/", routes![external::managed, managed_two])
.manage(MyType)
.manage(100i32)
.manage(100u32);
}

View File

@ -0,0 +1,42 @@
#![feature(plugin)]
#![plugin(rocket_codegen)]
#![allow(dead_code)]
#![deny(unmounted_route)]
extern crate rocket;
use rocket::State;
#[get("/one")]
fn one() { }
#[get("/two")]
fn two() { }
#[get("/three")]
fn three() { }
#[get("/four")]
fn four() { }
fn main() {
let instance = rocket::ignite()
.mount("/", routes![one]);
let other = instance.mount("/", routes![two]);
other.mount("/", routes![three])
.mount("/", routes![four]);
rocket::ignite()
.mount("/", routes![one])
.mount("/", routes![two])
.mount("/", routes![three])
.mount("/", routes![four]);
let a = rocket::ignite()
.mount("/", routes![one])
.mount("/", routes![two]);
let b = a.mount("/", routes![three])
.mount("/", routes![four]);
}

View File

@ -7,7 +7,7 @@ use rocket_contrib::Template;
#[test]
fn test_submit() {
let rocket = rocket::ignite().mount("/", routes![super::submit]);
let rocket = rocket::ignite().mount("/", routes![super::index, super::submit]);
let mut request = MockRequest::new(Method::Post, "/submit")
.header(ContentType::Form)
.body("message=Hello from Rocket!");
@ -21,7 +21,7 @@ fn test_submit() {
}
fn test_body(optional_cookie: Option<Cookie<'static>>, expected_body: String) {
let rocket = rocket::ignite().mount("/", routes![super::index]);
let rocket = rocket::ignite().mount("/", routes![super::index, super::submit]);
let mut request = MockRequest::new(Method::Get, "/");
// Attach a cookie if one is given.

View File

@ -6,7 +6,6 @@ workspace = "../../"
[dependencies]
rocket = { path = "../../lib" }
rocket_codegen = { path = "../../codegen" }
lazy_static = "*"
serde = "0.8"
serde_json = "0.8"
serde_derive = "0.8"

View File

@ -5,9 +5,8 @@ extern crate rocket;
extern crate serde_json;
#[macro_use] extern crate diesel;
#[macro_use] extern crate diesel_codegen;
#[macro_use] extern crate lazy_static;
#[macro_use] extern crate rocket_contrib;
#[macro_use] extern crate serde_derive;
extern crate rocket_contrib;
mod static_files;
mod task;