From 7af947a032bd2e6f6df6b903b40f142cd7b8d9e0 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 6 Sep 2020 12:26:53 -0400 Subject: Add consuming modifier to lvalues that are passed by value and not Copy --- crates/ide/src/syntax_highlighting.rs | 41 ++++++++++++++++++++-- .../test_data/highlighting.html | 13 +++---- crates/ide/src/syntax_highlighting/tests.rs | 13 +++---- 3 files changed, 53 insertions(+), 14 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/syntax_highlighting.rs b/crates/ide/src/syntax_highlighting.rs index 25d6f7abd..4fa9e638a 100644 --- a/crates/ide/src/syntax_highlighting.rs +++ b/crates/ide/src/syntax_highlighting.rs @@ -13,8 +13,8 @@ use rustc_hash::FxHashMap; use syntax::{ ast::{self, HasFormatSpecifier}, AstNode, AstToken, Direction, NodeOrToken, SyntaxElement, - SyntaxKind::*, - TextRange, WalkEvent, T, + SyntaxKind::{self, *}, + SyntaxNode, TextRange, WalkEvent, T, }; use crate::FileId; @@ -454,6 +454,23 @@ fn macro_call_range(macro_call: &ast::MacroCall) -> Option { Some(TextRange::new(range_start, range_end)) } +/// Returns true if the parent nodes of `node` all match the `SyntaxKind`s in `kinds` exactly. +fn parents_match(mut node: SyntaxNode, mut kinds: &[SyntaxKind]) -> bool { + while let (Some(parent), [kind, rest @ ..]) = (&node.parent(), kinds) { + if parent.kind() != *kind { + return false; + } + + // FIXME: Would be nice to get parent out of the match, but binding by-move and by-value + // in the same pattern is unstable: rust-lang/rust#68354. + node = node.parent().unwrap(); + kinds = rest; + } + + // Only true if we matched all expected kinds + kinds.len() == 0 +} + fn highlight_element( sema: &Semantics, bindings_shadow_count: &mut FxHashMap, @@ -522,6 +539,26 @@ fn highlight_element( let mut h = highlight_def(db, def); + // When lvalues are passed as arguments and they're not Copy, then mark + // them as Consuming. + if parents_match( + name_ref.syntax().clone(), + &[PATH_SEGMENT, PATH, PATH_EXPR, ARG_LIST], + ) { + let lvalue_ty = if let Definition::Local(local) = &def { + Some(local.ty(db)) + } else if let Definition::SelfType(impl_def) = &def { + Some(impl_def.target_ty(db)) + } else { + None + }; + if let Some(lvalue_ty) = lvalue_ty { + if !lvalue_ty.is_copy(db) { + h |= HighlightModifier::Consuming; + } + } + } + if let Some(parent) = name_ref.syntax().parent() { if matches!(parent.kind(), FIELD_EXPR | RECORD_PAT_FIELD) { if let Definition::Field(field) = def { diff --git a/crates/ide/src/syntax_highlighting/test_data/highlighting.html b/crates/ide/src/syntax_highlighting/test_data/highlighting.html index d0df2e0ec..cade46348 100644 --- a/crates/ide/src/syntax_highlighting/test_data/highlighting.html +++ b/crates/ide/src/syntax_highlighting/test_data/highlighting.html @@ -61,8 +61,8 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd } impl Foo { - fn baz(mut self) -> i32 { - self.x + fn baz(mut self, f: Foo) -> i32 { + f.baz(self) } fn qux(&mut self) { @@ -80,8 +80,8 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd } impl FooCopy { - fn baz(self) -> u32 { - self.x + fn baz(self, f: FooCopy) -> u32 { + f.baz(self) } fn qux(&mut self) { @@ -144,14 +144,15 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd y; let mut foo = Foo { x, y: x }; + let foo2 = foo.clone(); foo.quop(); foo.qux(); - foo.baz(); + foo.baz(foo2); let mut copy = FooCopy { x }; copy.quop(); copy.qux(); - copy.baz(); + copy.baz(copy); } enum Option<T> { diff --git a/crates/ide/src/syntax_highlighting/tests.rs b/crates/ide/src/syntax_highlighting/tests.rs index 6f72a29bd..57d4e1252 100644 --- a/crates/ide/src/syntax_highlighting/tests.rs +++ b/crates/ide/src/syntax_highlighting/tests.rs @@ -35,8 +35,8 @@ impl Bar for Foo { } impl Foo { - fn baz(mut self) -> i32 { - self.x + fn baz(mut self, f: Foo) -> i32 { + f.baz(self) } fn qux(&mut self) { @@ -54,8 +54,8 @@ struct FooCopy { } impl FooCopy { - fn baz(self) -> u32 { - self.x + fn baz(self, f: FooCopy) -> u32 { + f.baz(self) } fn qux(&mut self) { @@ -118,14 +118,15 @@ fn main() { y; let mut foo = Foo { x, y: x }; + let foo2 = foo.clone(); foo.quop(); foo.qux(); - foo.baz(); + foo.baz(foo2); let mut copy = FooCopy { x }; copy.quop(); copy.qux(); - copy.baz(); + copy.baz(copy); } enum Option { -- cgit v1.2.3 From a1a7b07ad33b7dcadedc2af26c3a5f8ef3daca27 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 6 Sep 2020 14:34:01 -0400 Subject: Fix handling of consuming self, refactor shared logic into a single function --- crates/ide/src/syntax_highlighting.rs | 62 ++++++++++++---------- .../test_data/highlighting.html | 2 +- 2 files changed, 34 insertions(+), 30 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/syntax_highlighting.rs b/crates/ide/src/syntax_highlighting.rs index 4fa9e638a..d9fc25d88 100644 --- a/crates/ide/src/syntax_highlighting.rs +++ b/crates/ide/src/syntax_highlighting.rs @@ -4,7 +4,7 @@ mod injection; #[cfg(test)] mod tests; -use hir::{Name, Semantics, VariantDef}; +use hir::{Local, Name, Semantics, VariantDef}; use ide_db::{ defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass}, RootDatabase, @@ -14,7 +14,7 @@ use syntax::{ ast::{self, HasFormatSpecifier}, AstNode, AstToken, Direction, NodeOrToken, SyntaxElement, SyntaxKind::{self, *}, - SyntaxNode, TextRange, WalkEvent, T, + SyntaxNode, SyntaxToken, TextRange, WalkEvent, T, }; use crate::FileId; @@ -455,7 +455,7 @@ fn macro_call_range(macro_call: &ast::MacroCall) -> Option { } /// Returns true if the parent nodes of `node` all match the `SyntaxKind`s in `kinds` exactly. -fn parents_match(mut node: SyntaxNode, mut kinds: &[SyntaxKind]) -> bool { +fn parents_match(mut node: NodeOrToken, mut kinds: &[SyntaxKind]) -> bool { while let (Some(parent), [kind, rest @ ..]) = (&node.parent(), kinds) { if parent.kind() != *kind { return false; @@ -463,7 +463,7 @@ fn parents_match(mut node: SyntaxNode, mut kinds: &[SyntaxKind]) -> bool { // FIXME: Would be nice to get parent out of the match, but binding by-move and by-value // in the same pattern is unstable: rust-lang/rust#68354. - node = node.parent().unwrap(); + node = node.parent().unwrap().into(); kinds = rest; } @@ -471,6 +471,15 @@ fn parents_match(mut node: SyntaxNode, mut kinds: &[SyntaxKind]) -> bool { kinds.len() == 0 } +fn is_consumed_lvalue( + node: NodeOrToken, + local: &Local, + db: &RootDatabase, +) -> bool { + // When lvalues are passed as arguments and they're not Copy, then mark them as Consuming. + parents_match(node, &[PATH_SEGMENT, PATH, PATH_EXPR, ARG_LIST]) && !local.ty(db).is_copy(db) +} + fn highlight_element( sema: &Semantics, bindings_shadow_count: &mut FxHashMap, @@ -539,23 +548,9 @@ fn highlight_element( let mut h = highlight_def(db, def); - // When lvalues are passed as arguments and they're not Copy, then mark - // them as Consuming. - if parents_match( - name_ref.syntax().clone(), - &[PATH_SEGMENT, PATH, PATH_EXPR, ARG_LIST], - ) { - let lvalue_ty = if let Definition::Local(local) = &def { - Some(local.ty(db)) - } else if let Definition::SelfType(impl_def) = &def { - Some(impl_def.target_ty(db)) - } else { - None - }; - if let Some(lvalue_ty) = lvalue_ty { - if !lvalue_ty.is_copy(db) { - h |= HighlightModifier::Consuming; - } + if let Definition::Local(local) = &def { + if is_consumed_lvalue(name_ref.syntax().clone().into(), local, db) { + h |= HighlightModifier::Consuming; } } @@ -682,21 +677,30 @@ fn highlight_element( .and_then(ast::SelfParam::cast) .and_then(|p| p.mut_token()) .is_some(); - // closure to enforce lazyness - let self_path = || { - sema.resolve_path(&element.parent()?.parent().and_then(ast::Path::cast)?) - }; + let self_path = &element + .parent() + .as_ref() + .and_then(SyntaxNode::parent) + .and_then(ast::Path::cast) + .and_then(|p| sema.resolve_path(&p)); + let mut h = HighlightTag::SelfKeyword.into(); if self_param_is_mut - || matches!(self_path(), + || matches!(self_path, Some(hir::PathResolution::Local(local)) if local.is_self(db) && (local.is_mut(db) || local.ty(db).is_mutable_reference()) ) { - HighlightTag::SelfKeyword | HighlightModifier::Mutable - } else { - HighlightTag::SelfKeyword.into() + h |= HighlightModifier::Mutable + } + + if let Some(hir::PathResolution::Local(local)) = self_path { + if is_consumed_lvalue(element, &local, db) { + h |= HighlightModifier::Consuming; + } } + + h } T![ref] => element .parent() diff --git a/crates/ide/src/syntax_highlighting/test_data/highlighting.html b/crates/ide/src/syntax_highlighting/test_data/highlighting.html index cade46348..cde42024c 100644 --- a/crates/ide/src/syntax_highlighting/test_data/highlighting.html +++ b/crates/ide/src/syntax_highlighting/test_data/highlighting.html @@ -62,7 +62,7 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd impl Foo { fn baz(mut self, f: Foo) -> i32 { - f.baz(self) + f.baz(self) } fn qux(&mut self) { -- cgit v1.2.3