From a39c0493a11cda3fed3c1e61099938b996886349 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 13 May 2024 12:51:57 +0200 Subject: [PATCH] Render literal escaping errors in hovers --- crates/hir-def/src/hir.rs | 6 +- crates/hir-expand/src/builtin_fn_macro.rs | 6 +- crates/ide-assists/src/handlers/raw_string.rs | 8 +- .../src/handlers/replace_string_with_char.rs | 2 +- crates/ide/src/goto_definition.rs | 6 +- crates/ide/src/hover.rs | 2 +- crates/ide/src/hover/render.rs | 10 +- crates/ide/src/syntax_highlighting/escape.rs | 4 +- crates/ide/src/syntax_highlighting/inject.rs | 2 +- crates/syntax/src/ast/token_ext.rs | 107 ++++++++---------- 10 files changed, 71 insertions(+), 82 deletions(-) diff --git a/crates/hir-def/src/hir.rs b/crates/hir-def/src/hir.rs index ac0caaf0dc..2f7ebbfec1 100644 --- a/crates/hir-def/src/hir.rs +++ b/crates/hir-def/src/hir.rs @@ -136,15 +136,15 @@ impl From for Literal { Literal::Float(FloatTypeWrapper::new(lit.value().unwrap_or(Default::default())), ty) } LiteralKind::ByteString(bs) => { - let text = bs.value().map(Box::from).unwrap_or_else(Default::default); + let text = bs.value().map_or_else(|_| Default::default(), Box::from); Literal::ByteString(text) } LiteralKind::String(s) => { - let text = s.value().map(Box::from).unwrap_or_else(Default::default); + let text = s.value().map_or_else(|_| Default::default(), Box::from); Literal::String(text) } LiteralKind::CString(s) => { - let text = s.value().map(Box::from).unwrap_or_else(Default::default); + let text = s.value().map_or_else(|_| Default::default(), Box::from); Literal::CString(text) } LiteralKind::Byte(b) => { diff --git a/crates/hir-expand/src/builtin_fn_macro.rs b/crates/hir-expand/src/builtin_fn_macro.rs index 4d6fe6db39..ba96ab6cc2 100644 --- a/crates/hir-expand/src/builtin_fn_macro.rs +++ b/crates/hir-expand/src/builtin_fn_macro.rs @@ -441,21 +441,21 @@ fn unquote_str(lit: &tt::Literal) -> Option<(String, Span)> { let span = lit.span; let lit = ast::make::tokens::literal(&lit.to_string()); let token = ast::String::cast(lit)?; - token.value().map(|it| (it.into_owned(), span)) + token.value().ok().map(|it| (it.into_owned(), span)) } fn unquote_char(lit: &tt::Literal) -> Option<(char, Span)> { let span = lit.span; let lit = ast::make::tokens::literal(&lit.to_string()); let token = ast::Char::cast(lit)?; - token.value().zip(Some(span)) + token.value().ok().zip(Some(span)) } fn unquote_byte_string(lit: &tt::Literal) -> Option<(Vec, Span)> { let span = lit.span; let lit = ast::make::tokens::literal(&lit.to_string()); let token = ast::ByteString::cast(lit)?; - token.value().map(|it| (it.into_owned(), span)) + token.value().ok().map(|it| (it.into_owned(), span)) } fn compile_error_expand( diff --git a/crates/ide-assists/src/handlers/raw_string.rs b/crates/ide-assists/src/handlers/raw_string.rs index 63db606336..5a197f23d0 100644 --- a/crates/ide-assists/src/handlers/raw_string.rs +++ b/crates/ide-assists/src/handlers/raw_string.rs @@ -25,7 +25,7 @@ pub(crate) fn make_raw_string(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt if token.is_raw() { return None; } - let value = token.value()?; + let value = token.value().ok()?; let target = token.syntax().text_range(); acc.add( AssistId("make_raw_string", AssistKind::RefactorRewrite), @@ -64,7 +64,7 @@ pub(crate) fn make_usual_string(acc: &mut Assists, ctx: &AssistContext<'_>) -> O if !token.is_raw() { return None; } - let value = token.value()?; + let value = token.value().ok()?; let target = token.syntax().text_range(); acc.add( AssistId("make_usual_string", AssistKind::RefactorRewrite), @@ -398,12 +398,12 @@ string"###; } #[test] - fn remove_hash_doesnt_work() { + fn remove_hash_does_not_work() { check_assist_not_applicable(remove_hash, r#"fn f() { let s = $0"random string"; }"#); } #[test] - fn remove_hash_no_hash_doesnt_work() { + fn remove_hash_no_hash_does_not_work() { check_assist_not_applicable(remove_hash, r#"fn f() { let s = $0r"random string"; }"#); } diff --git a/crates/ide-assists/src/handlers/replace_string_with_char.rs b/crates/ide-assists/src/handlers/replace_string_with_char.rs index 6310981ccc..a48b20acbc 100644 --- a/crates/ide-assists/src/handlers/replace_string_with_char.rs +++ b/crates/ide-assists/src/handlers/replace_string_with_char.rs @@ -25,7 +25,7 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // ``` pub(crate) fn replace_string_with_char(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let token = ctx.find_token_syntax_at_offset(STRING).and_then(ast::String::cast)?; - let value = token.value()?; + let value = token.value().ok()?; let target = token.syntax().text_range(); if value.chars().take(2).count() != 1 { diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index ddeeca5f7b..76b80fcefa 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -123,7 +123,7 @@ fn try_lookup_include_path( { return None; } - let path = token.value()?; + let path = token.value().ok()?; let file_id = sema.db.resolve_path(AnchoredPath { anchor: file_id, path: &path })?; let size = sema.db.file_text(file_id).len().try_into().ok()?; @@ -179,11 +179,11 @@ fn try_filter_trait_item_definition( AssocItem::Const(..) | AssocItem::TypeAlias(..) => { let trait_ = assoc.implemented_trait(db)?; let name = def.name(db)?; - let discri_value = discriminant(&assoc); + let discriminant_value = discriminant(&assoc); trait_ .items(db) .iter() - .filter(|itm| discriminant(*itm) == discri_value) + .filter(|itm| discriminant(*itm) == discriminant_value) .find_map(|itm| (itm.name(db)? == name).then(|| itm.try_to_nav(db)).flatten()) .map(|it| it.collect()) } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 6b73afe08e..bdb56081c3 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -15,7 +15,7 @@ use ide_db::{ FxIndexSet, RootDatabase, }; use itertools::{multizip, Itertools}; -use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxNode, T}; +use syntax::{ast, AstNode, SyntaxKind::*, SyntaxNode, T}; use crate::{ doc_links::token_as_doc_comment, diff --git a/crates/ide/src/hover/render.rs b/crates/ide/src/hover/render.rs index 674b32a7f5..e392a8ec38 100644 --- a/crates/ide/src/hover/render.rs +++ b/crates/ide/src/hover/render.rs @@ -533,11 +533,11 @@ pub(super) fn literal(sema: &Semantics<'_, RootDatabase>, token: SyntaxToken) -> let value = match_ast! { match token { - ast::String(string) => Ok(string.value()?.to_string()), - ast::ByteString(string) => Ok(format_args!("{:?}", string.value()?).to_string()), - ast::CString(string) => Ok(std::str::from_utf8(string.value()?.as_ref()).ok()?.to_owned()), - ast::Char(char) => Ok(char.value()?.to_string()), - ast::Byte(byte) => Ok(format!("0x{:X}", byte.value()?)), + ast::String(string) => string.value().as_ref().map_err(|e| format!("{e:?}")).map(ToString::to_string), + ast::ByteString(string) => string.value().as_ref().map_err(|e| format!("{e:?}")).map(|it| format!("{it:?}")), + ast::CString(string) => string.value().as_ref().map_err(|e| format!("{e:?}")).map(|it| std::str::from_utf8(it).map_or_else(|e| format!("{e:?}"), ToOwned::to_owned)), + ast::Char(char) => char .value().as_ref().map_err(|e| format!("{e:?}")).map(ToString::to_string), + ast::Byte(byte) => byte .value().as_ref().map_err(|e| format!("{e:?}")).map(|it| format!("0x{it:X}")), ast::FloatNumber(num) => { let (text, _) = num.split_into_parts(); let text = text.replace('_', ""); diff --git a/crates/ide/src/syntax_highlighting/escape.rs b/crates/ide/src/syntax_highlighting/escape.rs index 5913ca5e45..0439e509d2 100644 --- a/crates/ide/src/syntax_highlighting/escape.rs +++ b/crates/ide/src/syntax_highlighting/escape.rs @@ -25,7 +25,7 @@ pub(super) fn highlight_escape_string( } pub(super) fn highlight_escape_char(stack: &mut Highlights, char: &Char, start: TextSize) { - if char.value().is_none() { + if char.value().is_err() { // We do not emit invalid escapes highlighting here. The lexer would likely be in a bad // state and this token contains junks, since `'` is not a reliable delimiter (consider // lifetimes). Nonetheless, parser errors should already be emitted. @@ -48,7 +48,7 @@ pub(super) fn highlight_escape_char(stack: &mut Highlights, char: &Char, start: } pub(super) fn highlight_escape_byte(stack: &mut Highlights, byte: &Byte, start: TextSize) { - if byte.value().is_none() { + if byte.value().is_err() { // See `highlight_escape_char` for why no error highlighting here. return; } diff --git a/crates/ide/src/syntax_highlighting/inject.rs b/crates/ide/src/syntax_highlighting/inject.rs index 6bf13ffd06..10d867fe2b 100644 --- a/crates/ide/src/syntax_highlighting/inject.rs +++ b/crates/ide/src/syntax_highlighting/inject.rs @@ -30,7 +30,7 @@ pub(super) fn ra_fixture( if !active_parameter.ident().map_or(false, |name| name.text().starts_with("ra_fixture")) { return None; } - let value = literal.value()?; + let value = literal.value().ok()?; if let Some(range) = literal.open_quote_text_range() { hl.add(HlRange { range, highlight: HlTag::StringLiteral.into(), binding_hash: None }) diff --git a/crates/syntax/src/ast/token_ext.rs b/crates/syntax/src/ast/token_ext.rs index 7cd1f1550b..16599881d6 100644 --- a/crates/syntax/src/ast/token_ext.rs +++ b/crates/syntax/src/ast/token_ext.rs @@ -6,7 +6,7 @@ use std::{ }; use rustc_lexer::unescape::{ - unescape_byte, unescape_char, unescape_mixed, unescape_unicode, MixedUnit, Mode, + unescape_byte, unescape_char, unescape_mixed, unescape_unicode, EscapeError, MixedUnit, Mode, }; use crate::{ @@ -180,10 +180,7 @@ pub trait IsString: AstToken { fn close_quote_text_range(&self) -> Option { self.quote_offsets().map(|it| it.quotes.1) } - fn escaped_char_ranges( - &self, - cb: &mut dyn FnMut(TextRange, Result), - ) { + fn escaped_char_ranges(&self, cb: &mut dyn FnMut(TextRange, Result)) { let text_range_no_quotes = match self.text_range_between_quotes() { Some(it) => it, None => return, @@ -212,20 +209,17 @@ impl IsString for ast::String { } impl ast::String { - pub fn value(&self) -> Option> { - if self.is_raw() { - let text = self.text(); - let text = - &text[self.text_range_between_quotes()? - self.syntax().text_range().start()]; - return Some(Cow::Borrowed(text)); - } - + pub fn value(&self) -> Result, EscapeError> { let text = self.text(); - let text = &text[self.text_range_between_quotes()? - self.syntax().text_range().start()]; + let text_range = self.text_range_between_quotes().ok_or(EscapeError::LoneSlash)?; + let text = &text[text_range - self.syntax().text_range().start()]; + if self.is_raw() { + return Ok(Cow::Borrowed(text)); + } let mut buf = String::new(); let mut prev_end = 0; - let mut has_error = false; + let mut has_error = None; unescape_unicode(text, Self::MODE, &mut |char_range, unescaped_char| match ( unescaped_char, buf.capacity() == 0, @@ -239,13 +233,13 @@ impl ast::String { buf.push_str(&text[..prev_end]); buf.push(c); } - (Err(_), _) => has_error = true, + (Err(e), _) => has_error = Some(e), }); match (has_error, buf.capacity() == 0) { - (true, _) => None, - (false, true) => Some(Cow::Borrowed(text)), - (false, false) => Some(Cow::Owned(buf)), + (Some(e), _) => Err(e), + (None, true) => Ok(Cow::Borrowed(text)), + (None, false) => Ok(Cow::Owned(buf)), } } } @@ -256,20 +250,17 @@ impl IsString for ast::ByteString { } impl ast::ByteString { - pub fn value(&self) -> Option> { - if self.is_raw() { - let text = self.text(); - let text = - &text[self.text_range_between_quotes()? - self.syntax().text_range().start()]; - return Some(Cow::Borrowed(text.as_bytes())); - } - + pub fn value(&self) -> Result, EscapeError> { let text = self.text(); - let text = &text[self.text_range_between_quotes()? - self.syntax().text_range().start()]; + let text_range = self.text_range_between_quotes().ok_or(EscapeError::LoneSlash)?; + let text = &text[text_range - self.syntax().text_range().start()]; + if self.is_raw() { + return Ok(Cow::Borrowed(text.as_bytes())); + } let mut buf: Vec = Vec::new(); let mut prev_end = 0; - let mut has_error = false; + let mut has_error = None; unescape_unicode(text, Self::MODE, &mut |char_range, unescaped_char| match ( unescaped_char, buf.capacity() == 0, @@ -283,13 +274,13 @@ impl ast::ByteString { buf.extend_from_slice(text[..prev_end].as_bytes()); buf.push(c as u8); } - (Err(_), _) => has_error = true, + (Err(e), _) => has_error = Some(e), }); match (has_error, buf.capacity() == 0) { - (true, _) => None, - (false, true) => Some(Cow::Borrowed(text.as_bytes())), - (false, false) => Some(Cow::Owned(buf)), + (Some(e), _) => Err(e), + (None, true) => Ok(Cow::Borrowed(text.as_bytes())), + (None, false) => Ok(Cow::Owned(buf)), } } } @@ -298,10 +289,7 @@ impl IsString for ast::CString { const RAW_PREFIX: &'static str = "cr"; const MODE: Mode = Mode::CStr; - fn escaped_char_ranges( - &self, - cb: &mut dyn FnMut(TextRange, Result), - ) { + fn escaped_char_ranges(&self, cb: &mut dyn FnMut(TextRange, Result)) { let text_range_no_quotes = match self.text_range_between_quotes() { Some(it) => it, None => return, @@ -322,20 +310,17 @@ impl IsString for ast::CString { } impl ast::CString { - pub fn value(&self) -> Option> { - if self.is_raw() { - let text = self.text(); - let text = - &text[self.text_range_between_quotes()? - self.syntax().text_range().start()]; - return Some(Cow::Borrowed(text.as_bytes())); - } - + pub fn value(&self) -> Result, EscapeError> { let text = self.text(); - let text = &text[self.text_range_between_quotes()? - self.syntax().text_range().start()]; + let text_range = self.text_range_between_quotes().ok_or(EscapeError::LoneSlash)?; + let text = &text[text_range - self.syntax().text_range().start()]; + if self.is_raw() { + return Ok(Cow::Borrowed(text.as_bytes())); + } let mut buf = Vec::new(); let mut prev_end = 0; - let mut has_error = false; + let mut has_error = None; let extend_unit = |buf: &mut Vec, unit: MixedUnit| match unit { MixedUnit::Char(c) => buf.extend(c.encode_utf8(&mut [0; 4]).as_bytes()), MixedUnit::HighByte(b) => buf.push(b), @@ -353,13 +338,13 @@ impl ast::CString { buf.extend(text[..prev_end].as_bytes()); extend_unit(&mut buf, u); } - (Err(_), _) => has_error = true, + (Err(e), _) => has_error = Some(e), }); match (has_error, buf.capacity() == 0) { - (true, _) => None, - (false, true) => Some(Cow::Borrowed(text.as_bytes())), - (false, false) => Some(Cow::Owned(buf)), + (Some(e), _) => Err(e), + (None, true) => Ok(Cow::Borrowed(text.as_bytes())), + (None, false) => Ok(Cow::Owned(buf)), } } } @@ -478,34 +463,34 @@ impl Radix { } impl ast::Char { - pub fn value(&self) -> Option { + pub fn value(&self) -> Result { let mut text = self.text(); if text.starts_with('\'') { text = &text[1..]; } else { - return None; + return Err(EscapeError::ZeroChars); } if text.ends_with('\'') { text = &text[0..text.len() - 1]; } - unescape_char(text).ok() + unescape_char(text) } } impl ast::Byte { - pub fn value(&self) -> Option { + pub fn value(&self) -> Result { let mut text = self.text(); if text.starts_with("b\'") { text = &text[2..]; } else { - return None; + return Err(EscapeError::ZeroChars); } if text.ends_with('\'') { text = &text[0..text.len() - 1]; } - unescape_byte(text).ok() + unescape_byte(text) } } @@ -559,7 +544,10 @@ mod tests { fn check_string_value<'a>(lit: &str, expected: impl Into>) { assert_eq!( - ast::String { syntax: make::tokens::literal(&format!("\"{lit}\"")) }.value().as_deref(), + ast::String { syntax: make::tokens::literal(&format!("\"{lit}\"")) } + .value() + .as_deref() + .ok(), expected.into() ); } @@ -584,7 +572,8 @@ bcde", "abcde", assert_eq!( ast::ByteString { syntax: make::tokens::literal(&format!("b\"{lit}\"")) } .value() - .as_deref(), + .as_deref() + .ok(), expected.into().map(|value| &value[..]) ); }