4 files changed, 45 insertions, 31 deletions
diff --git a/src/librustc_save_analysis/json_api_dumper.rs b/src/librustc_save_analysis/json_api_dumper.rs
index 1f65efbf9dc..5c687646134 100644
--- a/src/librustc_save_analysis/json_api_dumper.rs
+++ b/src/librustc_save_analysis/json_api_dumper.rs
@@ -15,7 +15,7 @@ use rustc_serialize::json::as_json;
 use external_data::*;
 use data::{VariableKind, Visibility};
 use dump::Dump;
-use json_dumper::id_from_def_id;
+use id_from_def_id;
 
 use rls_data::{Analysis, Import, ImportKind, Def, DefKind, CratePreludeData};
 
diff --git a/src/librustc_save_analysis/json_dumper.rs b/src/librustc_save_analysis/json_dumper.rs
index 2832eb505ec..58df612c687 100644
--- a/src/librustc_save_analysis/json_dumper.rs
+++ b/src/librustc_save_analysis/json_dumper.rs
@@ -10,7 +10,6 @@
 
 use std::io::Write;
 
-use rustc::hir::def_id::DefId;
 use rustc_serialize::json::as_json;
 
 use rls_data::{self, Id, Analysis, Import, ImportKind, Def, DefKind, Ref, RefKind, MacroRef,
@@ -20,6 +19,7 @@ use rls_span::{Column, Row};
 use external_data::*;
 use data::VariableKind;
 use dump::Dump;
+use id_from_def_id;
 
 pub struct JsonDumper<O: DumpOutput> {
     result: Analysis,
@@ -163,15 +163,6 @@ impl<'b, O: DumpOutput + 'b> Dump for JsonDumper<O> {
 // method, but not the supplied method). In both cases, we are currently
 // ignoring it.
 
-// DefId::index is a newtype and so the JSON serialisation is ugly. Therefore
-// we use our own Id which is the same, but without the newtype.
-pub fn id_from_def_id(id: DefId) -> Id {
-    Id {
-        krate: id.krate.as_u32(),
-        index: id.index.as_u32(),
-    }
-}
-
 impl Into<Import> for ExternCrateData {
     fn into(self) -> Import {
         Import {
diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs
index 06309a4f79e..b6fafe3ee52 100644
--- a/src/librustc_save_analysis/lib.rs
+++ b/src/librustc_save_analysis/lib.rs
@@ -1026,6 +1026,20 @@ fn escape(s: String) -> String {
 
 // Helper function to determine if a span came from a
 // macro expansion or syntax extension.
-pub fn generated_code(span: Span) -> bool {
+fn generated_code(span: Span) -> bool {
     span.ctxt != NO_EXPANSION || span == DUMMY_SP
 }
+
+// DefId::index is a newtype and so the JSON serialisation is ugly. Therefore
+// we use our own Id which is the same, but without the newtype.
+fn id_from_def_id(id: DefId) -> rls_data::Id {
+    rls_data::Id {
+        krate: id.krate.as_u32(),
+        index: id.index.as_u32(),
+    }
+}
+
+fn id_from_node_id(id: NodeId, scx: &SaveContext) -> rls_data::Id {
+    let def_id = scx.tcx.hir.local_def_id(id);
+    id_from_def_id(def_id)
+}
diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs
index 0dd0112a1b6..6b734eb5248 100644
--- a/src/librustc_save_analysis/sig.rs
+++ b/src/librustc_save_analysis/sig.rs
@@ -8,16 +8,38 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// FIXME? None of these include visibility info.
-// Large outstanding things - where clauses, defs/refs for generics
-// paresable - each sig ends with `;` of ` {}`
+// A signature is a string representation of an item's type signature, excluding
+// any body. It also includes ids for any defs or refs in the signature. For
+// example:
+//
+// ```
+// fn foo(x: String) {
+//     println!("{}", x);   
+// }
+// ```
+// The signature string is something like "fn foo(x: String) {}" and the signature
+// will have defs for `foo` and `x` and a ref for `String`.
+//
+// All signature text should parse in the correct context (i.e., in a module or
+// impl, etc.). Clients may want to trim trailing `{}` or `;`. The text of a
+// signature is not guaranteed to be stable (it may improve or change as the
+// syntax changes, or whitespace or punctuation may change). It is also likely
+// not to be pretty - no attempt is made to prettify the text. It is recommended
+// that clients run the text through Rustfmt.
+//
+// This module generates Signatures for items by walking the AST and looking up
+// references.
+//
+// Signatures do not include visibility info. I'm not sure if this is a feature
+// or an ommission (FIXME).
+//
+// FIXME where clauses need implementing, defs/refs in generics are mostly missing.
 
-use SaveContext;
+use {SaveContext, id_from_def_id, id_from_node_id};
 
-use rls_data::{Signature, SigElement, Id};
+use rls_data::{Signature, SigElement};
 
 use rustc::hir::def::Def;
-use rustc::hir::def_id::DefId;
 use syntax::ast::{self, NodeId};
 use syntax::print::pprust;
 
@@ -26,19 +48,6 @@ pub fn item_signature(item: &ast::Item, scx: &SaveContext) -> Option<Signature>
     item.make(0, None, scx).ok()
 }
 
-// TODO dup from json_dumper
-fn id_from_def_id(id: DefId) -> Id {
-    Id {
-        krate: id.krate.as_u32(),
-        index: id.index.as_u32(),
-    }
-}
-
-fn id_from_node_id(id: NodeId, scx: &SaveContext) -> Id {
-    let def_id = scx.tcx.hir.local_def_id(id);
-    id_from_def_id(def_id)
-}
-
 type Result = ::std::result::Result<Signature, &'static str>;
 
 trait Sig {