From 9349353e044c06a5baa9cfbcfd98e7d329b0bc0e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 3 Nov 2020 19:49:15 +0100 Subject: Fix overflow panic in convert_interger_literal assist This also seizes the opportunity to move integer literal parsing to the syntax crate, were it logically belongs. Note though that this is still done in an ad hoc manner -- we probably should split kitchen sink ast::Literal into a separate APIs for strings, ints, etc --- .../src/handlers/convert_integer_literal.rs | 206 +++++++-------------- crates/syntax/src/ast.rs | 2 +- crates/syntax/src/ast/expr_ext.rs | 65 +++++++ 3 files changed, 137 insertions(+), 136 deletions(-) diff --git a/crates/assists/src/handlers/convert_integer_literal.rs b/crates/assists/src/handlers/convert_integer_literal.rs index ea35e833a..c8af80701 100644 --- a/crates/assists/src/handlers/convert_integer_literal.rs +++ b/crates/assists/src/handlers/convert_integer_literal.rs @@ -1,4 +1,4 @@ -use syntax::{ast, AstNode, SmolStr}; +use syntax::{ast, ast::Radix, AstNode}; use crate::{AssistContext, AssistId, AssistKind, Assists, GroupLabel}; @@ -15,37 +15,34 @@ use crate::{AssistContext, AssistId, AssistKind, Assists, GroupLabel}; // ``` pub(crate) fn convert_integer_literal(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let literal = ctx.find_node_at_offset::()?; + let (radix, value) = literal.int_value()?; + let range = literal.syntax().text_range(); let group_id = GroupLabel("Convert integer base".into()); - let suffix = match literal.kind() { ast::LiteralKind::IntNumber { suffix } => suffix, _ => return None, }; - let suffix_len = suffix.as_ref().map(|s| s.len()).unwrap_or(0); - let raw_literal_text = literal.syntax().to_string(); - - // Gets the literal's text without the type suffix and without underscores. - let literal_text = raw_literal_text - .chars() - .take(raw_literal_text.len() - suffix_len) - .filter(|c| *c != '_') - .collect::(); - let literal_base = IntegerLiteralBase::identify(&literal_text)?; - - for base in IntegerLiteralBase::bases() { - if *base == literal_base { + + for &target_radix in Radix::ALL { + if target_radix == radix { continue; } - let mut converted = literal_base.convert(&literal_text, base); - - let label = if let Some(suffix) = &suffix { - format!("Convert {} ({}) to {}", &literal_text, suffix, &converted) - } else { - format!("Convert {} to {}", &literal_text, &converted) + let mut converted = match target_radix { + Radix::Binary => format!("0b{:b}", value), + Radix::Octal => format!("0o{:o}", value), + Radix::Decimal => value.to_string(), + Radix::Hexadecimal => format!("0x{:X}", value), }; + let label = format!( + "Convert {} to {}{}", + literal, + converted, + suffix.as_deref().unwrap_or_default() + ); + // Appends the type suffix back into the new literal if it exists. if let Some(suffix) = &suffix { converted.push_str(&suffix); @@ -63,79 +60,11 @@ pub(crate) fn convert_integer_literal(acc: &mut Assists, ctx: &AssistContext) -> Some(()) } -#[derive(Debug, PartialEq, Eq)] -enum IntegerLiteralBase { - Binary, - Octal, - Decimal, - Hexadecimal, -} - -impl IntegerLiteralBase { - fn identify(literal_text: &str) -> Option { - // We cannot express a literal in anything other than decimal in under 3 characters, so we return here if possible. - if literal_text.len() < 3 && literal_text.chars().all(|c| c.is_digit(10)) { - return Some(Self::Decimal); - } - - let base = match &literal_text[..2] { - "0b" => Self::Binary, - "0o" => Self::Octal, - "0x" => Self::Hexadecimal, - _ => Self::Decimal, - }; - - // Checks that all characters after the base prefix are all valid digits for that base. - if literal_text[base.prefix_len()..].chars().all(|c| c.is_digit(base.base())) { - Some(base) - } else { - None - } - } - - fn convert(&self, literal_text: &str, to: &IntegerLiteralBase) -> String { - let digits = &literal_text[self.prefix_len()..]; - let value = u128::from_str_radix(digits, self.base()).unwrap(); - - match to { - Self::Binary => format!("0b{:b}", value), - Self::Octal => format!("0o{:o}", value), - Self::Decimal => value.to_string(), - Self::Hexadecimal => format!("0x{:X}", value), - } - } - - const fn base(&self) -> u32 { - match self { - Self::Binary => 2, - Self::Octal => 8, - Self::Decimal => 10, - Self::Hexadecimal => 16, - } - } - - const fn prefix_len(&self) -> usize { - match self { - Self::Decimal => 0, - _ => 2, - } - } - - const fn bases() -> &'static [IntegerLiteralBase] { - &[ - IntegerLiteralBase::Binary, - IntegerLiteralBase::Octal, - IntegerLiteralBase::Decimal, - IntegerLiteralBase::Hexadecimal, - ] - } -} - #[cfg(test)] mod tests { + use crate::tests::{check_assist_by_label, check_assist_not_applicable, check_assist_target}; use super::*; - use crate::tests::{check_assist_by_label, check_assist_target}; #[test] fn binary_target() { @@ -317,21 +246,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b1111101000;", - "Convert 1000 to 0b1111101000", + "Convert 1_00_0 to 0b1111101000", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o1750;", - "Convert 1000 to 0o1750", + "Convert 1_00_0 to 0o1750", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0x3E8;", - "Convert 1000 to 0x3E8", + "Convert 1_00_0 to 0x3E8", ); } @@ -343,21 +272,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b1010;", - "Convert 10 to 0b1010", + "Convert 1_0 to 0b1010", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o12;", - "Convert 10 to 0o12", + "Convert 1_0 to 0o12", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xA;", - "Convert 10 to 0xA", + "Convert 1_0 to 0xA", ); } @@ -369,21 +298,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b11111111;", - "Convert 0xFF to 0b11111111", + "Convert 0x_F_F to 0b11111111", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o377;", - "Convert 0xFF to 0o377", + "Convert 0x_F_F to 0o377", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255;", - "Convert 0xFF to 255", + "Convert 0x_F_F to 255", ); } @@ -395,21 +324,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0o377;", - "Convert 0b11111111 to 0o377", + "Convert 0b1111_1111 to 0o377", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255;", - "Convert 0b11111111 to 255", + "Convert 0b1111_1111 to 255", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xFF;", - "Convert 0b11111111 to 0xFF", + "Convert 0b1111_1111 to 0xFF", ); } @@ -421,21 +350,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b11111111;", - "Convert 0o377 to 0b11111111", + "Convert 0o3_77 to 0b11111111", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255;", - "Convert 0o377 to 255", + "Convert 0o3_77 to 255", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xFF;", - "Convert 0o377 to 0xFF", + "Convert 0o3_77 to 0xFF", ); } @@ -447,21 +376,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b1111101000i32;", - "Convert 1000 (i32) to 0b1111101000", + "Convert 1000i32 to 0b1111101000i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o1750i32;", - "Convert 1000 (i32) to 0o1750", + "Convert 1000i32 to 0o1750i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0x3E8i32;", - "Convert 1000 (i32) to 0x3E8", + "Convert 1000i32 to 0x3E8i32", ); } @@ -473,21 +402,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b1010i32;", - "Convert 10 (i32) to 0b1010", + "Convert 10i32 to 0b1010i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o12i32;", - "Convert 10 (i32) to 0o12", + "Convert 10i32 to 0o12i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xAi32;", - "Convert 10 (i32) to 0xA", + "Convert 10i32 to 0xAi32", ); } @@ -499,21 +428,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b11111111i32;", - "Convert 0xFF (i32) to 0b11111111", + "Convert 0xFFi32 to 0b11111111i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o377i32;", - "Convert 0xFF (i32) to 0o377", + "Convert 0xFFi32 to 0o377i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255i32;", - "Convert 0xFF (i32) to 255", + "Convert 0xFFi32 to 255i32", ); } @@ -525,21 +454,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0o377i32;", - "Convert 0b11111111 (i32) to 0o377", + "Convert 0b11111111i32 to 0o377i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255i32;", - "Convert 0b11111111 (i32) to 255", + "Convert 0b11111111i32 to 255i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xFFi32;", - "Convert 0b11111111 (i32) to 0xFF", + "Convert 0b11111111i32 to 0xFFi32", ); } @@ -551,21 +480,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b11111111i32;", - "Convert 0o377 (i32) to 0b11111111", + "Convert 0o377i32 to 0b11111111i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255i32;", - "Convert 0o377 (i32) to 255", + "Convert 0o377i32 to 255i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xFFi32;", - "Convert 0o377 (i32) to 0xFF", + "Convert 0o377i32 to 0xFFi32", ); } @@ -577,21 +506,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b1111101000i32;", - "Convert 1000 (i32) to 0b1111101000", + "Convert 1_00_0i32 to 0b1111101000i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o1750i32;", - "Convert 1000 (i32) to 0o1750", + "Convert 1_00_0i32 to 0o1750i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0x3E8i32;", - "Convert 1000 (i32) to 0x3E8", + "Convert 1_00_0i32 to 0x3E8i32", ); } @@ -603,21 +532,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b1010i32;", - "Convert 10 (i32) to 0b1010", + "Convert 1_0i32 to 0b1010i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o12i32;", - "Convert 10 (i32) to 0o12", + "Convert 1_0i32 to 0o12i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xAi32;", - "Convert 10 (i32) to 0xA", + "Convert 1_0i32 to 0xAi32", ); } @@ -629,21 +558,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b11111111i32;", - "Convert 0xFF (i32) to 0b11111111", + "Convert 0x_F_Fi32 to 0b11111111i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0o377i32;", - "Convert 0xFF (i32) to 0o377", + "Convert 0x_F_Fi32 to 0o377i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255i32;", - "Convert 0xFF (i32) to 255", + "Convert 0x_F_Fi32 to 255i32", ); } @@ -655,21 +584,21 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0o377i32;", - "Convert 0b11111111 (i32) to 0o377", + "Convert 0b1111_1111i32 to 0o377i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255i32;", - "Convert 0b11111111 (i32) to 255", + "Convert 0b1111_1111i32 to 255i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xFFi32;", - "Convert 0b11111111 (i32) to 0xFF", + "Convert 0b1111_1111i32 to 0xFFi32", ); } @@ -681,21 +610,28 @@ mod tests { convert_integer_literal, before, "const _: i32 = 0b11111111i32;", - "Convert 0o377 (i32) to 0b11111111", + "Convert 0o3_77i32 to 0b11111111i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 255i32;", - "Convert 0o377 (i32) to 255", + "Convert 0o3_77i32 to 255i32", ); check_assist_by_label( convert_integer_literal, before, "const _: i32 = 0xFFi32;", - "Convert 0o377 (i32) to 0xFF", + "Convert 0o3_77i32 to 0xFFi32", ); } + + #[test] + fn convert_overflowing_literal() { + let before = "const _: i32 = + 111111111111111111111111111111111111111111111111111111111111111111111111<|>;"; + check_assist_not_applicable(convert_integer_literal, before); + } } diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index 8a0e3d27b..a16ac6a7c 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -16,7 +16,7 @@ use crate::{ }; pub use self::{ - expr_ext::{ArrayExprKind, BinOp, Effect, ElseBranch, LiteralKind, PrefixOp, RangeOp}, + expr_ext::{ArrayExprKind, BinOp, Effect, ElseBranch, LiteralKind, PrefixOp, Radix, RangeOp}, generated::{nodes::*, tokens::*}, node_ext::{ AttrKind, FieldKind, NameOrNameRef, PathSegmentKind, SelfParamKind, SlicePatComponents, diff --git a/crates/syntax/src/ast/expr_ext.rs b/crates/syntax/src/ast/expr_ext.rs index f5ba87223..3aff01e83 100644 --- a/crates/syntax/src/ast/expr_ext.rs +++ b/crates/syntax/src/ast/expr_ext.rs @@ -358,6 +358,71 @@ impl ast::Literal { _ => unreachable!(), } } + + // FIXME: should probably introduce string token type? + // https://github.com/rust-analyzer/rust-analyzer/issues/6308 + pub fn int_value(&self) -> Option<(Radix, u128)> { + let suffix = match self.kind() { + LiteralKind::IntNumber { suffix } => suffix, + _ => return None, + }; + + let token = self.token(); + let mut text = token.text().as_str(); + text = &text[..text.len() - suffix.map_or(0, |it| it.len())]; + + let buf; + if text.contains("_") { + buf = text.replace('_', ""); + text = buf.as_str(); + }; + + let radix = Radix::identify(text)?; + let digits = &text[radix.prefix_len()..]; + let value = u128::from_str_radix(digits, radix as u32).ok()?; + Some((radix, value)) + } +} + +#[derive(Debug, PartialEq, Eq, Copy, Clone)] +pub enum Radix { + Binary = 2, + Octal = 8, + Decimal = 10, + Hexadecimal = 16, +} + +impl Radix { + pub const ALL: &'static [Radix] = + &[Radix::Binary, Radix::Octal, Radix::Decimal, Radix::Hexadecimal]; + + fn identify(literal_text: &str) -> Option { + // We cannot express a literal in anything other than decimal in under 3 characters, so we return here if possible. + if literal_text.len() < 3 && literal_text.chars().all(|c| c.is_digit(10)) { + return Some(Self::Decimal); + } + + let res = match &literal_text[..2] { + "0b" => Radix::Binary, + "0o" => Radix::Octal, + "0x" => Radix::Hexadecimal, + _ => Radix::Decimal, + }; + + // Checks that all characters after the base prefix are all valid digits for that base. + if literal_text[res.prefix_len()..].chars().all(|c| c.is_digit(res as u32)) { + Some(res) + } else { + None + } + } + + const fn prefix_len(&self) -> usize { + match self { + Self::Decimal => 0, + _ => 2, + } + } } #[derive(Debug, Clone, PartialEq, Eq)] -- cgit v1.2.3