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

Various cosmetic improvements #1314

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions rls/src/actions/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,8 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! 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 @@ -98,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 @@ -131,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 @@ -244,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 @@ -275,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 @@ -304,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 @@ -332,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 @@ -419,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 @@ -477,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 @@ -675,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
21 changes: 6 additions & 15 deletions rls/src/actions/format.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! 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 @@ -22,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 @@ -90,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
72 changes: 30 additions & 42 deletions rls/src/actions/hover.rs
Original file line number Diff line number Diff line change
@@ -1,30 +1,20 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

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 @@ -63,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 @@ -124,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 @@ -134,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 @@ -148,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 @@ -181,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 @@ -224,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 @@ -328,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 @@ -383,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 @@ -471,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 @@ -555,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 @@ -636,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 @@ -692,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 @@ -721,7 +710,7 @@ 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
// Avoid creating tooltip text that is exactly the item being hovered over
// 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))
});
Expand All @@ -741,7 +730,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 @@ -765,7 +754,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 @@ -789,11 +778,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 @@ -865,8 +854,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
10 changes: 0 additions & 10 deletions rls/src/actions/notifications.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! One-way notifications that the RLS receives from the client.

use crate::actions::{FileWatch, InitActionContext, VersionOrdering};
Expand Down
Loading