mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #7051 #7219 #7245 #7246

Browse Source 
7051: Check dbg! macro in tidy_test r=matklad a=edwin0cheng

Same as `check_todo` but for dbg! macro

r? @matklad 

7219: Refactor rename name checking r=matklad a=Veykril

Improves the user facing error messages a bit and prevents renaming to `_` when the name is referenced as this would change source to not compile anymore since `_` is only a pattern, not a proper identifier.

7245: Encourage gifs r=matklad a=matklad

bors r+
🤖

7246: Unfreeze cargo_metadata r=matklad a=kjeremy

It now pulls in a newer version of semver-parser.

This does add a dependency on `cargo-platform` in the interest of correctness.

Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: kjeremy <kjeremy@gmail.com>
This commit is contained in:
bors[bot] 2021-01-11 13:37:28 +00:00 committed by GitHub
commit 8ad325a806
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 215 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
Cargo.lock generated
View File

@ -127,12 +127,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "08c48aae112d48ed9f069b33538ea9e3e90aa263cfa3d1c24309612b1f7472de"
[[package]]
name = "cargo_metadata"
version = "0.12.0"
name = "cargo-platform"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d5a5f7b42f606b7f23674f6f4d877628350682bc40687d3fae65679a58d55345"
checksum = "0226944a63d1bf35a3b5f948dd7c59e263db83695c9e8bffc4037de02e30f1d7"
dependencies = [
"serde",
]
[[package]]
name = "cargo_metadata"
version = "0.12.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "11a47b6286279a9998588ef7050d1ebc2500c69892a557c90fe5d071c64415dc"
dependencies = [
"cargo-platform",
"semver",
"semver-parser",
"serde",
"serde_json",
]

8
crates/completion/src/completions/dot.rs
View File

@ -373,20 +373,20 @@ fn foo(a: A) {
fn macro_expansion_resilient() {
check(
r#"
macro_rules! dbg {
macro_rules! d {
() => {};
($val:expr) => {
match $val { tmp => { tmp } }
};
// Trailing comma with single argument is ignored
($val:expr,) => { $crate::dbg!($val) };
($val:expr,) => { $crate::d!($val) };
($($val:expr),+ $(,)?) => {
($($crate::dbg!($val)),+,)
($($crate::d!($val)),+,)
};
}
struct A { the_field: u32 }
fn foo(a: A) {
dbg!(a.$0)
d!(a.$0)
}
"#,
expect![[r#"

2
crates/flycheck/Cargo.toml
View File

@ -12,7 +12,7 @@ doctest = false
[dependencies]
crossbeam-channel = "0.5.0"
log = "0.4.8"
cargo_metadata = "=0.12.0"
cargo_metadata = "0.12.2"
serde_json = "1.0.48"
jod-thread = "0.1.1"

7
crates/ide/src/hover.rs
View File

@ -175,12 +175,7 @@ fn to_action(nav_target: NavigationTarget) -> HoverAction {
Definition::SelfType(it) => it.target_ty(db).as_adt(),
_ => None,
}?;
match adt {
Adt::Struct(it) => it.try_to_nav(db),
Adt::Union(it) => it.try_to_nav(db),
Adt::Enum(it) => it.try_to_nav(db),
}
.map(to_action)
adt.try_to_nav(db).map(to_action)
}
fn runnable_action(

234
crates/ide/src/references/rename.rs
View File

@ -14,16 +14,17 @@
use syntax::{
algo::find_node_at_offset,
ast::{self, NameOwner},
lex_single_syntax_kind, match_ast, AstNode, SyntaxKind, SyntaxNode, SyntaxToken,
lex_single_syntax_kind, match_ast, AstNode, SyntaxKind, SyntaxNode, SyntaxToken, T,
};
use test_utils::mark;
use text_edit::TextEdit;
use crate::{
references::find_all_refs, FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind,
FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind, ReferenceSearchResult,
SourceChange, SourceFileEdit, TextRange, TextSize,
};
type RenameResult<T> = Result<T, RenameError>;
#[derive(Debug)]
pub struct RenameError(pub(crate) String);
@ -35,24 +36,30 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
impl Error for RenameError {}
macro_rules! format_err {
($fmt:expr) => {RenameError(format!($fmt))};
($fmt:expr, $($arg:tt)+) => {RenameError(format!($fmt, $($arg)+))}
}
macro_rules! bail {
($($tokens:tt)*) => {return Err(format_err!($($tokens)*))}
}
pub(crate) fn prepare_rename(
db: &RootDatabase,
position: FilePosition,
) -> Result<RangeInfo<()>, RenameError> {
) -> RenameResult<RangeInfo<()>> {
let sema = Semantics::new(db);
let source_file = sema.parse(position.file_id);
let syntax = source_file.syntax();
if let Some(module) = find_module_at_offset(&sema, position, syntax) {
rename_mod(&sema, position, module, "dummy")
} else if let Some(self_token) =
syntax.token_at_offset(position.offset).find(|t| t.kind() == SyntaxKind::SELF_KW)
syntax.token_at_offset(position.offset).find(|t| t.kind() == T![self])
{
rename_self_to_param(&sema, position, self_token, "dummy")
} else {
let range = match find_all_refs(&sema, position, None) {
Some(RangeInfo { range, .. }) => range,
None => return Err(RenameError("No references found at position".to_string())),
};
let RangeInfo { range, .. } = find_all_refs(&sema, position)?;
Ok(RangeInfo::new(range, SourceChange::from(vec![])))
}
.map(|info| RangeInfo::new(info.range, ()))
@ -62,7 +69,7 @@ pub(crate) fn rename(
db: &RootDatabase,
position: FilePosition,
new_name: &str,
) -> Result<RangeInfo<SourceChange>, RenameError> {
) -> RenameResult<RangeInfo<SourceChange>> {
let sema = Semantics::new(db);
rename_with_semantics(&sema, position, new_name)
}
@ -71,42 +78,18 @@ pub(crate) fn rename_with_semantics(
sema: &Semantics<RootDatabase>,
position: FilePosition,
new_name: &str,
) -> Result<RangeInfo<SourceChange>, RenameError> {
let is_lifetime_name = match lex_single_syntax_kind(new_name) {
Some(res) => match res {
(SyntaxKind::IDENT, _) => false,
(SyntaxKind::UNDERSCORE, _) => false,
(SyntaxKind::SELF_KW, _) => return rename_to_self(&sema, position),
(SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => true,
(SyntaxKind::LIFETIME_IDENT, _) => {
return Err(RenameError(format!(
"Invalid name `{0}`: Cannot rename lifetime to {0}",
new_name
)))
}
(_, Some(syntax_error)) => {
return Err(RenameError(format!("Invalid name `{}`: {}", new_name, syntax_error)))
}
(_, None) => {
return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name)))
}
},
None => return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))),
};
) -> RenameResult<RangeInfo<SourceChange>> {
let source_file = sema.parse(position.file_id);
let syntax = source_file.syntax();
// this is here to prevent lifetime renames from happening on modules and self
if is_lifetime_name {
rename_reference(&sema, position, new_name, is_lifetime_name)
} else if let Some(module) = find_module_at_offset(&sema, position, syntax) {
if let Some(module) = find_module_at_offset(&sema, position, syntax) {
rename_mod(&sema, position, module, new_name)
} else if let Some(self_token) =
syntax.token_at_offset(position.offset).find(|t| t.kind() == SyntaxKind::SELF_KW)
syntax.token_at_offset(position.offset).find(|t| t.kind() == T![self])
{
rename_self_to_param(&sema, position, self_token, new_name)
} else {
rename_reference(&sema, position, new_name, is_lifetime_name)
rename_reference(&sema, position, new_name)
}
}
@ -127,6 +110,33 @@ pub(crate) fn will_rename_file(
Some(change)
}
#[derive(Debug, PartialEq)]
enum IdentifierKind {
Ident,
Lifetime,
ToSelf,
Underscore,
}
fn check_identifier(new_name: &str) -> RenameResult<IdentifierKind> {
match lex_single_syntax_kind(new_name) {
Some(res) => match res {
(SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident),
(T![_], _) => Ok(IdentifierKind::Underscore),
(T![self], _) => Ok(IdentifierKind::ToSelf),
(SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => {
Ok(IdentifierKind::Lifetime)
}
(SyntaxKind::LIFETIME_IDENT, _) => {
bail!("Invalid name `{0}`: Cannot rename lifetime to {0}", new_name)
}
(_, Some(syntax_error)) => bail!("Invalid name `{}`: {}", new_name, syntax_error),
(_, None) => bail!("Invalid name `{}`: not an identifier", new_name),
},
None => bail!("Invalid name `{}`: not an identifier", new_name),
}
}
fn find_module_at_offset(
sema: &Semantics<RootDatabase>,
position: FilePosition,
@ -155,6 +165,14 @@ fn find_module_at_offset(
Some(module)
}
fn find_all_refs(
sema: &Semantics<RootDatabase>,
position: FilePosition,
) -> RenameResult<RangeInfo<ReferenceSearchResult>> {
crate::references::find_all_refs(sema, position, None)
.ok_or_else(|| format_err!("No references found at position"))
}
fn source_edit_from_reference(
sema: &Semantics<RootDatabase>,
reference: Reference,
@ -223,7 +241,10 @@ fn rename_mod(
position: FilePosition,
module: Module,
new_name: &str,
) -> Result<RangeInfo<SourceChange>, RenameError> {
) -> RenameResult<RangeInfo<SourceChange>> {
if IdentifierKind::Ident != check_identifier(new_name)? {
bail!("Invalid name `{0}`: cannot rename module to {0}", new_name);
}
let mut source_file_edits = Vec::new();
let mut file_system_edits = Vec::new();
@ -254,8 +275,7 @@ fn rename_mod(
source_file_edits.push(edit);
}
let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
.ok_or_else(|| RenameError("No references found at position".to_string()))?;
let RangeInfo { range, info: refs } = find_all_refs(sema, position)?;
let ref_edits = refs
.references
.into_iter()
@ -274,27 +294,26 @@ fn rename_to_self(
let (fn_def, fn_ast) = find_node_at_offset::<ast::Fn>(syn, position.offset)
.and_then(|fn_ast| sema.to_def(&fn_ast).zip(Some(fn_ast)))
.ok_or_else(|| RenameError("No surrounding method declaration found".to_string()))?;
.ok_or_else(|| format_err!("No surrounding method declaration found"))?;
let param_range = fn_ast
.param_list()
.and_then(|p| p.params().next())
.ok_or_else(|| RenameError("Method has no parameters".to_string()))?
.ok_or_else(|| format_err!("Method has no parameters"))?
.syntax()
.text_range();
if !param_range.contains(position.offset) {
return Err(RenameError("Only the first parameter can be self".to_string()));
bail!("Only the first parameter can be self");
}
let impl_block = find_node_at_offset::<ast::Impl>(syn, position.offset)
.and_then(|def| sema.to_def(&def))
.ok_or_else(|| RenameError("No impl block found for function".to_string()))?;
.ok_or_else(|| format_err!("No impl block found for function"))?;
if fn_def.self_param(sema.db).is_some() {
return Err(RenameError("Method already has a self parameter".to_string()));
bail!("Method already has a self parameter");
}
let params = fn_def.assoc_fn_params(sema.db);
let first_param =
params.first().ok_or_else(|| RenameError("Method has no parameters".into()))?;
let first_param = params.first().ok_or_else(|| format_err!("Method has no parameters"))?;
let first_param_ty = first_param.ty();
let impl_ty = impl_block.target_ty(sema.db);
let (ty, self_param) = if impl_ty.remove_ref().is_some() {
@ -307,18 +326,17 @@ fn rename_to_self(
};
if ty != impl_ty {
return Err(RenameError("Parameter type differs from impl block type".to_string()));
bail!("Parameter type differs from impl block type");
}
let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
.ok_or_else(|| RenameError("No reference found at position".to_string()))?;
let RangeInfo { range, info: refs } = find_all_refs(sema, position)?;
let (param_ref, usages): (Vec<Reference>, Vec<Reference>) = refs
.into_iter()
.partition(|reference| param_range.intersect(reference.file_range.range).is_some());
if param_ref.is_empty() {
return Err(RenameError("Parameter to rename not found".to_string()));
bail!("Parameter to rename not found");
}
let mut edits = usages
@ -367,12 +385,22 @@ fn rename_self_to_param(
self_token: SyntaxToken,
new_name: &str,
) -> Result<RangeInfo<SourceChange>, RenameError> {
let ident_kind = check_identifier(new_name)?;
match ident_kind {
IdentifierKind::Lifetime => bail!("Invalid name `{}`: not an identifier", new_name),
IdentifierKind::ToSelf => {
// no-op
mark::hit!(rename_self_to_self);
return Ok(RangeInfo::new(self_token.text_range(), SourceChange::default()));
}
_ => (),
}
let source_file = sema.parse(position.file_id);
let syn = source_file.syntax();
let text = sema.db.file_text(position.file_id);
let fn_def = find_node_at_offset::<ast::Fn>(syn, position.offset)
.ok_or_else(|| RenameError("No surrounding method declaration found".to_string()))?;
.ok_or_else(|| format_err!("No surrounding method declaration found"))?;
let search_range = fn_def.syntax().text_range();
let mut edits: Vec<SourceFileEdit> = vec![];
@ -382,12 +410,10 @@ fn rename_self_to_param(
if !search_range.contains_inclusive(offset) {
continue;
}
if let Some(ref usage) =
syn.token_at_offset(offset).find(|t| t.kind() == SyntaxKind::SELF_KW)
{
if let Some(ref usage) = syn.token_at_offset(offset).find(|t| t.kind() == T![self]) {
let edit = if let Some(ref self_param) = ast::SelfParam::cast(usage.parent()) {
text_edit_from_self_param(syn, self_param, new_name)
.ok_or_else(|| RenameError("No target type found".to_string()))?
.ok_or_else(|| format_err!("No target type found"))?
} else {
TextEdit::replace(usage.text_range(), String::from(new_name))
};
@ -395,6 +421,10 @@ fn rename_self_to_param(
}
}
if edits.len() > 1 && ident_kind == IdentifierKind::Underscore {
bail!("Cannot rename reference to `_` as it is being referenced multiple times");
}
let range = ast::SelfParam::cast(self_token.parent())
.map_or(self_token.text_range(), |p| p.syntax().text_range());
@ -405,24 +435,34 @@ fn rename_reference(
sema: &Semantics<RootDatabase>,
position: FilePosition,
new_name: &str,
is_lifetime_name: bool,
) -> Result<RangeInfo<SourceChange>, RenameError> {
let RangeInfo { range, info: refs } = match find_all_refs(sema, position, None) {
Some(range_info) => range_info,
None => return Err(RenameError("No references found at position".to_string())),
};
let ident_kind = check_identifier(new_name)?;
let RangeInfo { range, info: refs } = find_all_refs(sema, position)?;
match (refs.declaration.kind == ReferenceKind::Lifetime, is_lifetime_name) {
(true, false) => {
return Err(RenameError(format!(
"Invalid name `{}`: not a lifetime identifier",
new_name
)))
match (ident_kind, &refs.declaration.kind) {
(IdentifierKind::ToSelf, ReferenceKind::Lifetime)
| (IdentifierKind::Underscore, ReferenceKind::Lifetime)
| (IdentifierKind::Ident, ReferenceKind::Lifetime) => {
mark::hit!(rename_not_a_lifetime_ident_ref);
bail!("Invalid name `{}`: not a lifetime identifier", new_name)
}
(false, true) => {
return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name)))
(IdentifierKind::Lifetime, ReferenceKind::Lifetime) => mark::hit!(rename_lifetime),
(IdentifierKind::Lifetime, _) => {
mark::hit!(rename_not_an_ident_ref);
bail!("Invalid name `{}`: not an identifier", new_name)
}
_ => (),
(IdentifierKind::ToSelf, ReferenceKind::SelfKw) => {
unreachable!("rename_self_to_param should've been called instead")
}
(IdentifierKind::ToSelf, _) => {
mark::hit!(rename_to_self);
return rename_to_self(sema, position);
}
(IdentifierKind::Underscore, _) if !refs.references.is_empty() => {
mark::hit!(rename_underscore_multiple);
bail!("Cannot rename reference to `_` as it is being referenced multiple times")
}
(IdentifierKind::Ident, _) | (IdentifierKind::Underscore, _) => mark::hit!(rename_ident),
}
let edit = refs
@ -430,10 +470,6 @@ fn rename_reference(
.map(|reference| source_edit_from_reference(sema, reference, new_name))
.collect::<Vec<_>>();
if edit.is_empty() {
return Err(RenameError("No references found at position".to_string()));
}
Ok(RangeInfo::new(range, SourceChange::from(edit)))
}
@ -462,10 +498,12 @@ fn check(new_name: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
text_edit_builder.replace(indel.delete, indel.insert);
}
}
let mut result = analysis.file_text(file_id.unwrap()).unwrap().to_string();
if let Some(file_id) = file_id {
let mut result = analysis.file_text(file_id).unwrap().to_string();
text_edit_builder.finish().apply(&mut result);
assert_eq_text!(ra_fixture_after, &*result);
}
}
Err(err) => {
if ra_fixture_after.starts_with("error:") {
let error_message = ra_fixture_after
@ -530,6 +568,7 @@ fn test_rename_to_invalid_identifier3() {
#[test]
fn test_rename_to_invalid_identifier_lifetime() {
mark::check!(rename_not_an_ident_ref);
check(
"'foo",
r#"fn main() { let i$0 = 1; }"#,
@ -539,6 +578,7 @@ fn test_rename_to_invalid_identifier_lifetime() {
#[test]
fn test_rename_to_invalid_identifier_lifetime2() {
mark::check!(rename_not_a_lifetime_ident_ref);
check(
"foo",
r#"fn main<'a>(_: &'a$0 ()) {}"#,
@ -546,8 +586,28 @@ fn test_rename_to_invalid_identifier_lifetime2() {
);
}
#[test]
fn test_rename_to_underscore_invalid() {
mark::check!(rename_underscore_multiple);
check(
"_",
r#"fn main(foo$0: ()) {foo;}"#,
"error: Cannot rename reference to `_` as it is being referenced multiple times",
);
}
#[test]
fn test_rename_mod_invalid() {
check(
"'foo",
r#"mod foo$0 {}"#,
"error: Invalid name `'foo`: cannot rename module to 'foo",
);
}
#[test]
fn test_rename_for_local() {
mark::check!(rename_ident);
check(
"k",
r#"
@ -1178,6 +1238,7 @@ fn foo(f: foo::Foo) {
#[test]
fn test_parameter_to_self() {
mark::check!(rename_to_self);
check(
"self",
r#"
@ -1481,6 +1542,7 @@ fn foo(Foo { i: bar }: foo) -> i32 {
#[test]
fn test_rename_lifetimes() {
mark::check!(rename_lifetime);
check(
"'yeeee",
r#"
@ -1562,6 +1624,26 @@ fn foo<'a>() -> &'a () {
}
}
}
"#,
)
}
#[test]
fn test_self_to_self() {
mark::check!(rename_self_to_self);
check(
"self",
r#"
struct Foo;
impl Foo {
fn foo(self$0) {}
}
"#,
r#"
struct Foo;
impl Foo {
fn foo(self) {}
}
"#,
)
}

2
crates/proc_macro_srv/Cargo.toml
View File

@ -20,7 +20,7 @@ proc_macro_api = { path = "../proc_macro_api", version = "0.0.0" }
test_utils = { path = "../test_utils", version = "0.0.0" }
[dev-dependencies]
cargo_metadata = "=0.12.0"
cargo_metadata = "0.12.2"
# used as proc macro test targets
serde_derive = "1.0.106"

2
crates/project_model/Cargo.toml
View File

@ -12,7 +12,7 @@ doctest = false
[dependencies]
log = "0.4.8"
rustc-hash = "1.1.0"
cargo_metadata = "=0.12.0"
cargo_metadata = "0.12.2"
serde = { version = "1.0.106", features = ["derive"] }
serde_json = "1.0.48"
anyhow = "1.0.26"

2
docs/dev/style.md
View File

@ -78,6 +78,8 @@ Use original span for FileId
This makes it easier to prepare a changelog.
If the change adds a new user-visible functionality, consider recording a GIF with [peek](https://github.com/phw/peek) and pasting it into the PR description.
**Rationale:** clean history is potentially useful, but rarely used.
But many users read changelogs.

32
xtask/tests/tidy.rs
View File

@ -85,6 +85,7 @@ fn rust_files_are_tidy() {
for path in rust_files(&project_root().join("crates")) {
let text = read_file(&path).unwrap();
check_todo(&path, &text);
check_dbg(&path, &text);
check_trailing_ws(&path, &text);
deny_clippy(&path, &text);
tidy_docs.visit(&path, &text);
@ -94,8 +95,7 @@ fn rust_files_are_tidy() {
#[test]
fn check_merge_commits() {
let stdout =
dbg!(cmd!("git rev-list --merges --invert-grep --author 'bors\\[bot\\]' HEAD~19.."))
let stdout = cmd!("git rev-list --merges --invert-grep --author 'bors\\[bot\\]' HEAD~19..")
.read()
.unwrap();
if !stdout.is_empty() {
@ -224,7 +224,7 @@ fn check_licenses() {
fn check_todo(path: &Path, text: &str) {
let need_todo = &[
// This file itself obviously needs to use todo (<- like this!).
"tests/cli.rs",
"tests/tidy.rs",
// Some of our assists generate `todo!()`.
"handlers/add_turbo_fish.rs",
"handlers/generate_function.rs",
@ -252,6 +252,32 @@ fn check_todo(path: &Path, text: &str) {
}
}
fn check_dbg(path: &Path, text: &str) {
let need_dbg = &[
// This file itself obviously needs to use dbg.
"tests/tidy.rs",
// Assists to remove `dbg!()`
"handlers/remove_dbg.rs",
// We have .dbg postfix
"completion/src/completions/postfix.rs",
// The documentation in string literals may contain anything for its own purposes
"completion/src/lib.rs",
"completion/src/generated_lint_completions.rs",
// test for doc test for remove_dbg
"src/tests/generated.rs",
];
if need_dbg.iter().any(|p| path.ends_with(p)) {
return;
}
if text.contains("dbg!") {
panic!(
"\ndbg! macros should not be committed to the master branch,\n\
{}\n",
path.display(),
)
}
}
fn check_trailing_ws(path: &Path, text: &str) {
if is_exclude_dir(path, &["test_data"]) {
return;