Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Commit

Permalink
Various cosmetic improvements.
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexander Regueiro committed Feb 16, 2019
1 parent 990a668 commit 7cb1217
Show file tree
Hide file tree
Showing 22 changed files with 407 additions and 391 deletions.
31 changes: 16 additions & 15 deletions rls/src/actions/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Conversion of raw rustc-emitted JSON messages into LSP diagnostics.
//!
//! Data definitions for diagnostics can be found in the Rust compiler for:
//! 1. Internal diagnostics at src/librustc_errors/diagnostic.rs.
//! 2. Emitted JSON format at src/libsyntax/json.rs.
//! 1. Internal diagnostics at `src/librustc_errors/diagnostic.rs`.
//! 2. Emitted JSON format at `src/libsyntax/json.rs`.

use std::collections::HashMap;
use std::iter;
Expand Down Expand Up @@ -88,7 +88,8 @@ pub fn parse_diagnostics(

let mut diagnostics = HashMap::new();

// If the client doesn't support related information, emit separate diagnostics for secondary spans
// If the client doesn't support related information, emit separate diagnostics for
// secondary spans.
let diagnostic_spans = if related_information_support { &primaries } else { &message.spans };

for (path, diagnostic) in diagnostic_spans.iter().map(|span| {
Expand Down Expand Up @@ -121,8 +122,8 @@ pub fn parse_diagnostics(

let rls_span = {
let mut span = span;
// if span points to a macro, search through the expansions
// for a more useful source location
// If span points to a macro, search through the expansions
// for a more useful source location.
while span.file_name.ends_with(" macros>") && span.expansion.is_some() {
span = &span.expansion.as_ref().unwrap().span;
}
Expand Down Expand Up @@ -234,8 +235,8 @@ fn make_suggestions<'a>(
.collect();

// Suggestions are displayed at primary span, so if the change is somewhere
// else, be sure to specify that
// TODO: In theory this can even point to different files - does that happen in practice?
// else, be sure to specify that.
// TODO: In theory this can even point to different files -- does that happen in practice?
for suggestion in &mut suggestions {
if !suggestion.range.is_within(&primary_range) {
let line = suggestion.range.start.line + 1; // as 1-based
Expand Down Expand Up @@ -265,7 +266,7 @@ fn label_suggestion(span: &DiagnosticSpan, label: &str) -> Option<Suggestion> {

trait IsWithin {
/// Returns whether `other` is considered within `self`
/// note: a thing should be 'within' itself
/// NOTE: a thing should be 'within' itself.
fn is_within(&self, other: &Self) -> bool;
}

Expand Down Expand Up @@ -294,9 +295,9 @@ impl IsWithin for Range {
}
}

/// Tests for formatted messages from the compilers json output
/// run cargo with `--message-format=json` to generate the json for new tests and add .json
/// message files to '$FIXTURES_DIR/compiler_message/'
/// Tests for formatted messages from the compiler's JSON output.
/// Runs cargo with `--message-format=json` to generate the JSON for new tests and add JSON
/// message files to the `$FIXTURES_DIR/compiler_message/` directory.
#[cfg(test)]
mod diagnostic_message_test {
use super::*;
Expand All @@ -322,7 +323,7 @@ mod diagnostic_message_test {

pub(super) trait FileDiagnosticTestExt {
fn single_file_results(&self) -> &Vec<(Diagnostic, Vec<Suggestion>)>;
/// Returns (primary message, secondary messages)
/// Returns `(primary message, secondary messages)`.
fn to_messages(&self) -> Vec<(String, Vec<String>)>;
fn to_primary_messages(&self) -> Vec<String>;
fn to_secondary_messages(&self) -> Vec<String>;
Expand Down Expand Up @@ -409,7 +410,7 @@ mod diagnostic_message_test {
assert_eq!(messages[0].1, vec!["consider giving `v` a type", "cannot infer type for `T`"]);

// Check if we don't emit related information if it's not supported and
// if secondary spans are emitted as separate diagnostics
// if secondary spans are emitted as separate diagnostics.
let messages = parse_compiler_message(
&read_fixture("compiler_message/type-annotations-needed.json"),
false,
Expand Down Expand Up @@ -467,7 +468,7 @@ mod diagnostic_message_test {
cannot borrow mutably",
);

// note: consider message becomes a suggestion
// NOTE: 'consider' message becomes a suggestion.
assert_eq!(
messages[0].1,
vec!["consider changing this to `mut string`", "cannot borrow mutably",]
Expand Down Expand Up @@ -665,7 +666,7 @@ help: consider borrowing here: `&string`"#,
}
}

/// Tests for creating suggestions from the compilers json output
/// Tests for creating suggestions from the compilers JSON output.
#[cfg(test)]
mod diagnostic_suggestion_test {
use self::diagnostic_message_test::*;
Expand Down
11 changes: 6 additions & 5 deletions rls/src/actions/format.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Code formatting using Rustfmt - by default using statically-linked one or
//! Code formatting using Rustfmt -- by default using statically-linked one or
//! possibly running Rustfmt binary specified by the user.

use std::env::temp_dir;
Expand All @@ -12,12 +12,12 @@ use rand::{distributions, thread_rng, Rng};
use rustfmt_nightly::{Config, Input, Session};
use serde_json;

/// Specified which `rustfmt` to use.
/// Specifies which `rustfmt` to use.
#[derive(Clone)]
pub enum Rustfmt {
/// (Path to external `rustfmt`, cwd where it should be spawned at)
/// `(path to external `rustfmt`, current working directory to spawn at)`
External(PathBuf, PathBuf),
/// Statically linked `rustfmt`
/// Statically linked `rustfmt`.
Internal,
}

Expand Down Expand Up @@ -80,7 +80,8 @@ fn format_internal(input: String, config: Config) -> Result<String, String> {

match session.format(Input::Text(input)) {
Ok(report) => {
// Session::format returns Ok even if there are any errors, i.e., parsing errors.
// `Session::format` returns `Ok` even if there are any errors, i.e., parsing
// errors.
if session.has_operational_errors() || session.has_parsing_errors() {
debug!("reformat: format_input failed: has errors, report = {}", report);

Expand Down
76 changes: 45 additions & 31 deletions rls/src/actions/hover.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
use crate::actions::format::Rustfmt;
use crate::actions::requests;
use crate::actions::InitActionContext;
use crate::config::FmtConfig;
use crate::lsp_data::*;
use crate::server::ResponseError;
use std::path::{Path, PathBuf};

use home;
use log::*;
use racer;
use rls_analysis::{Def, DefKind};
use rls_span::{Column, Range, Row, Span, ZeroIndexed};
use rls_vfs::{self as vfs, Vfs};
use rustfmt_nightly::NewlineStyle;
use serde_derive::{Deserialize, Serialize};

use log::*;
use std::path::{Path, PathBuf};
use crate::actions::format::Rustfmt;
use crate::actions::requests;
use crate::actions::InitActionContext;
use crate::config::FmtConfig;
use crate::lsp_data::*;
use crate::server::ResponseError;

#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)]
pub struct Tooltip {
Expand Down Expand Up @@ -53,7 +53,7 @@ pub fn process_docs(docs: &str) -> String {
line.to_string()
};

// Racer sometimes pulls out comment block headers from the standard library
// Racer sometimes pulls out comment block headers from the standard library.
let ignore_slashes = line.starts_with("////");

let maybe_attribute = trimmed.starts_with("#[") || trimmed.starts_with("#![");
Expand Down Expand Up @@ -114,7 +114,7 @@ pub fn extract_docs(
let attr_start = line.starts_with("#[") || line.starts_with("#![");

if attr_start && line.ends_with(']') && !hit_top {
// Ignore single line attributes
// Ignore single-line attributes.
trace!(
"extract_docs: ignoring single-line attribute, next_row: {:?}, up: {}",
next_row,
Expand All @@ -124,7 +124,7 @@ pub fn extract_docs(
}

// Continue with the next line when transitioning out of a
// multi-line attribute
// multi-line attribute.
if attr_start || (line.ends_with(']') && !line.starts_with("//")) {
in_meta = !in_meta;
if !in_meta && !hit_top {
Expand All @@ -138,7 +138,7 @@ pub fn extract_docs(
}

if !hit_top && in_meta {
// Ignore milti-line attributes
// Ignore milti-line attributes.
trace!(
"extract_docs: ignoring multi-line attribute, next_row: {:?}, up: {}, in_meta: {}",
next_row,
Expand Down Expand Up @@ -171,7 +171,7 @@ pub fn extract_docs(
}

if hit_top {
// The top of the file was reached
// The top of the file was reached.
debug!(
"extract_docs: bailing out: prev_row == next_row; next_row = {:?}, up = {}",
next_row, up
Expand Down Expand Up @@ -214,8 +214,7 @@ fn extract_and_process_docs(vfs: &Vfs, file: &Path, row_start: Row<ZeroIndexed>)
.and_then(empty_to_none)
}

/// Extracts a function, method, struct, enum, or trait declaration
/// from source.
/// Extracts a function, method, struct, enum, or trait declaration from source.
pub fn extract_decl(
vfs: &Vfs,
file: &Path,
Expand Down Expand Up @@ -318,10 +317,10 @@ fn tooltip_struct_enum_union_trait(

let vfs = ctx.vfs.clone();
let fmt_config = ctx.fmt_config();
// We hover often so use the in-process one to speed things up
// We hover often, so use the in-process one to speed things up.
let fmt = Rustfmt::Internal;

// fallback in case source extration fails
// Fallback in case source extration fails.
let the_type = || match def.kind {
DefKind::Struct => format!("struct {}", def.name),
DefKind::Enum => format!("enum {}", def.name),
Expand Down Expand Up @@ -373,7 +372,7 @@ fn tooltip_function_method(

let vfs = ctx.vfs.clone();
let fmt_config = ctx.fmt_config();
// We hover often so use the in-process one to speed things up
// We hover often, so use the in-process one to speed things up.
let fmt = Rustfmt::Internal;

let the_type = || {
Expand Down Expand Up @@ -461,7 +460,7 @@ fn empty_to_none(s: String) -> Option<String> {
}
}

/// Extract and process source documentation for the give `def`.
/// Extracts and processes source documentation for the give `def`.
fn def_docs(def: &Def, vfs: &Vfs) -> Option<String> {
let save_analysis_docs = || empty_to_none(def.docs.trim().into());
extract_and_process_docs(&vfs, def.span.file.as_ref(), def.span.range.row_start)
Expand Down Expand Up @@ -545,7 +544,7 @@ fn skip_path_components<P: AsRef<Path>>(
})
}

/// Collapse parent directory references inside of paths.
/// Collapses parent directory references inside of paths.
///
/// # Example
///
Expand Down Expand Up @@ -626,12 +625,12 @@ fn racer_match_to_def(ctx: &InitActionContext, m: &racer::Match) -> Option<Def>
let contextstr_path = PathBuf::from(&contextstr);
let contextstr_path = collapse_parents(contextstr_path);

// Tidy up the module path
// Skips toolchains/$TOOLCHAIN/lib/rustlib/src/rust/src
// Tidy up the module path.
// Skips `toolchains/$TOOLCHAIN/lib/rustlib/src/rust/src`.
skip_path_components(&contextstr_path, rustup_home, 7)
// Skips /registry/src/git.luolix.top-1ecc6299db9ec823/
// Skips `/registry/src/git.luolix.top-1ecc6299db9ec823/`.
.or_else(|| skip_path_components(&contextstr_path, cargo_home, 3))
// Make the path relative to the root of the project, if possible
// Make the path relative to the root of the project, if possible.
.or_else(|| {
contextstr_path.strip_prefix(&ctx.current_project).ok().map(|x| x.to_owned())
})
Expand Down Expand Up @@ -682,7 +681,7 @@ fn racer_match_to_def(ctx: &InitActionContext, m: &racer::Match) -> Option<Def>
})
}

/// Use racer to synthesize a `Def` for the given `span`. If no appropriate
/// Uses racer to synthesize a `Def` for the given `span`. If no appropriate
/// match is found with coordinates, `None` is returned.
fn racer_def(ctx: &InitActionContext, span: &Span<ZeroIndexed>) -> Option<Def> {
let vfs = ctx.vfs.clone();
Expand Down Expand Up @@ -711,9 +710,25 @@ fn racer_def(ctx: &InitActionContext, span: &Span<ZeroIndexed>) -> Option<Def> {
let racer_match = racer::find_definition(file_path, location, &session);
trace!("racer_def: match: {:?}", racer_match);
racer_match
<<<<<<< HEAD:rls/src/actions/hover.rs
// Avoid creating tooltip text that is exactly the item being hovered over
.filter(|m| name.as_ref().map(|name| name != &m.contextstr).unwrap_or(true))
.and_then(|m| racer_match_to_def(ctx, &m))
||||||| parent of 2391136... Various cosmetic improvements.:src/actions/hover.rs
// Avoid creating tooltip text that is exactly the item being hovered over
.filter(|m| {
name.as_ref()
.map(|name| name != &m.contextstr)
.unwrap_or(true)
}).and_then(|m| racer_match_to_def(ctx, &m))
=======
// Avoid creating tooltip text that is exactly the item being hovered over.
.filter(|m| {
name.as_ref()
.map(|name| name != &m.contextstr)
.unwrap_or(true)
}).and_then(|m| racer_match_to_def(ctx, &m))
>>>>>>> 2391136... Various cosmetic improvements.:src/actions/hover.rs
});

let results = results.map_err(|_| {
Expand All @@ -731,7 +746,7 @@ fn format_object(rustfmt: Rustfmt, fmt_config: &FmtConfig, the_type: String) ->
config.set().newline_style(NewlineStyle::Unix);
let trimmed = the_type.trim();

// Normalize the ending for rustfmt
// Normalize the ending for rustfmt.
let object = if trimmed.ends_with(')') {
format!("{};", trimmed)
} else if trimmed.ends_with('}') || trimmed.ends_with(';') {
Expand All @@ -755,7 +770,7 @@ fn format_object(rustfmt: Rustfmt, fmt_config: &FmtConfig, the_type: String) ->
};

// If it's a tuple, remove the trailing ';' and hide non-pub components
// for pub types
// for pub types.
let result = if formatted.trim().ends_with(';') {
let mut decl = formatted.trim().trim_end_matches(';');
if let (Some(pos), true) = (decl.rfind('('), decl.ends_with(')')) {
Expand All @@ -779,11 +794,11 @@ fn format_object(rustfmt: Rustfmt, fmt_config: &FmtConfig, the_type: String) ->
decl.to_string()
}
} else {
// not a tuple
// Not a tuple.
decl.into()
}
} else {
// not a tuple or unit struct
// Not a tuple or unit struct.
formatted
};

Expand Down Expand Up @@ -855,8 +870,7 @@ pub fn tooltip(

let racer_fallback_enabled = ctx.config.lock().unwrap().racer_completion;

// Fallback to racer if the def was not available and
// racer is enabled.
// Fallback to racer if the def was not available and racer is enabled.
let hover_span_def = hover_span_def.or_else(|e| {
debug!("tooltip: racer_fallback_enabled: {}", racer_fallback_enabled);
if racer_fallback_enabled {
Expand Down
4 changes: 2 additions & 2 deletions rls/src/actions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,12 @@ impl InitActionContext {
self.build_queue.block_on_build();
}

/// Returns true if there are no builds pending or in progress.
/// Returns `true` if there are no builds pending or in progress.
fn build_ready(&self) -> bool {
self.build_queue.build_ready()
}

/// Returns true if there are no builds or post-build (analysis) tasks pending
/// Returns `true` if there are no builds or post-build (analysis) tasks pending
/// or in progress.
fn analysis_ready(&self) -> bool {
self.active_build_count.load(Ordering::SeqCst) == 0
Expand Down
Loading

0 comments on commit 7cb1217

Please sign in to comment.