various refactorings (#909)

* ran `cargo clippy --fix -- -Wclippy::use_self`

* refactoring

* refactored counting functions to use `std::iter::successors`
claude/debug-issue-865-crash-011CUd9ut241YRW738w3K84r
adamnemecek 2025-10-26 16:49:08 +07:00 committed by GitHub
parent 5df84a4e92
commit d615490493
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 97 additions and 125 deletions

@ -156,33 +156,20 @@ fn edge_between<'s, 'v>(before: &Vertex<'s, 'v>, after: &Vertex<'s, 'v>) -> Edge
/// What is the total number of AST nodes? /// What is the total number of AST nodes?
fn node_count(root: Option<&Syntax>) -> u32 { fn node_count(root: Option<&Syntax>) -> u32 {
let mut node = root; let iter = std::iter::successors(root, |node| node.next_sibling());
let mut count = 0;
while let Some(current_node) = node { iter.map(|node| match node {
let current_count = match current_node { Syntax::List {
Syntax::List { num_descendants, ..
num_descendants, .. } => *num_descendants,
} => *num_descendants, Syntax::Atom { .. } => 1,
Syntax::Atom { .. } => 1, })
}; .sum::<u32>()
count += current_count;
node = current_node.next_sibling();
}
count
} }
/// How many top-level AST nodes do we have? /// How many top-level AST nodes do we have?
fn tree_count(root: Option<&Syntax>) -> u32 { fn tree_count(root: Option<&Syntax>) -> u32 {
let mut node = root; std::iter::successors(root, |node| node.next_sibling()).count() as _
let mut count = 0;
while let Some(current_node) = node {
count += 1;
node = current_node.next_sibling();
}
count
} }
pub(crate) fn mark_syntax<'a>( pub(crate) fn mark_syntax<'a>(

@ -138,14 +138,14 @@ enum EnteredDelimiter<'s, 'v> {
impl fmt::Debug for EnteredDelimiter<'_, '_> { impl fmt::Debug for EnteredDelimiter<'_, '_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let desc = match self { let desc = match self {
EnteredDelimiter::PopEither((lhs_delims, rhs_delims)) => { Self::PopEither((lhs_delims, rhs_delims)) => {
format!( format!(
"PopEither(lhs count: {}, rhs count: {})", "PopEither(lhs count: {}, rhs count: {})",
lhs_delims.size(), lhs_delims.size(),
rhs_delims.size() rhs_delims.size()
) )
} }
EnteredDelimiter::PopBoth(_) => "PopBoth".to_owned(), Self::PopBoth(_) => "PopBoth".to_owned(),
}; };
f.write_str(&desc) f.write_str(&desc)
} }

@ -24,11 +24,11 @@ impl<'b, T> Stack<'b, T> {
self.head.map(|n| &n.val) self.head.map(|n| &n.val)
} }
pub(crate) fn pop(&self) -> Option<Stack<'b, T>> { pub(crate) fn pop(&self) -> Option<Self> {
self.head.map(|n| Self { head: n.next }) self.head.map(|n| Self { head: n.next })
} }
pub(crate) fn push(&self, v: T, alloc: &'b Bump) -> Stack<'b, T> { pub(crate) fn push(&self, v: T, alloc: &'b Bump) -> Self {
Self { Self {
head: Some(alloc.alloc(Node { head: Some(alloc.alloc(Node {
val: v, val: v,
@ -39,13 +39,7 @@ impl<'b, T> Stack<'b, T> {
// O(n) // O(n)
pub(crate) fn size(&self) -> usize { pub(crate) fn size(&self) -> usize {
let mut count = 0; std::iter::successors(self.head, |&n| n.next).count()
let mut node = &self.head;
while let Some(next) = node {
count += 1;
node = &next.next;
}
count
} }
pub(crate) fn is_empty(&self) -> bool { pub(crate) fn is_empty(&self) -> bool {

@ -66,7 +66,7 @@ impl Hunk {
)); ));
} }
Hunk { Self {
novel_lhs: self.novel_lhs.union(&other.novel_lhs).copied().collect(), novel_lhs: self.novel_lhs.union(&other.novel_lhs).copied().collect(),
novel_rhs: self.novel_rhs.union(&other.novel_rhs).copied().collect(), novel_rhs: self.novel_rhs.union(&other.novel_rhs).copied().collect(),
lines: deduped_lines, lines: deduped_lines,

@ -32,11 +32,7 @@ struct File<'f> {
} }
impl<'f> File<'f> { impl<'f> File<'f> {
fn with_sections( fn with_sections(language: &'f FileFormat, path: &'f str, chunks: Vec<Vec<Line<'f>>>) -> Self {
language: &'f FileFormat,
path: &'f str,
chunks: Vec<Vec<Line<'f>>>,
) -> File<'f> {
File { File {
language, language,
path, path,
@ -45,7 +41,7 @@ impl<'f> File<'f> {
} }
} }
fn with_status(language: &'f FileFormat, path: &'f str, status: Status) -> File<'f> { fn with_status(language: &'f FileFormat, path: &'f str, status: Status) -> Self {
File { File {
language, language,
path, path,
@ -201,7 +197,7 @@ struct Line<'l> {
} }
impl<'l> Line<'l> { impl<'l> Line<'l> {
fn new(lhs_number: Option<u32>, rhs_number: Option<u32>) -> Line<'l> { fn new(lhs_number: Option<u32>, rhs_number: Option<u32>) -> Self {
Line { Line {
lhs: lhs_number.map(Side::new), lhs: lhs_number.map(Side::new),
rhs: rhs_number.map(Side::new), rhs: rhs_number.map(Side::new),
@ -216,7 +212,7 @@ struct Side<'s> {
} }
impl<'s> Side<'s> { impl<'s> Side<'s> {
fn new(line_number: u32) -> Side<'s> { fn new(line_number: u32) -> Self {
Side { Side {
line_number, line_number,
changes: Vec::new(), changes: Vec::new(),
@ -259,15 +255,15 @@ impl Highlight {
}; };
match highlight { match highlight {
TokenKind::Delimiter => Highlight::Delimiter, TokenKind::Delimiter => Self::Delimiter,
TokenKind::Atom(atom) => match atom { TokenKind::Atom(atom) => match atom {
AtomKind::String(StringKind::StringLiteral) => Highlight::String, AtomKind::String(StringKind::StringLiteral) => Self::String,
AtomKind::String(StringKind::Text) => Highlight::Normal, AtomKind::String(StringKind::Text) => Self::Normal,
AtomKind::Keyword => Highlight::Keyword, AtomKind::Keyword => Self::Keyword,
AtomKind::Comment => Highlight::Comment, AtomKind::Comment => Self::Comment,
AtomKind::Type => Highlight::Type, AtomKind::Type => Self::Type,
AtomKind::Normal => Highlight::Normal, AtomKind::Normal => Self::Normal,
AtomKind::TreeSitterError => Highlight::TreeSitterError, AtomKind::TreeSitterError => Self::TreeSitterError,
}, },
} }
} }

@ -26,7 +26,7 @@ pub(crate) enum BackgroundColor {
impl BackgroundColor { impl BackgroundColor {
pub(crate) fn is_dark(self) -> bool { pub(crate) fn is_dark(self) -> bool {
matches!(self, BackgroundColor::Dark) matches!(self, Self::Dark)
} }
} }

@ -398,7 +398,7 @@ pub(crate) enum FileArgument {
impl FileArgument { impl FileArgument {
pub(crate) fn permissions(&self) -> Option<FilePermissions> { pub(crate) fn permissions(&self) -> Option<FilePermissions> {
match self { match self {
FileArgument::NamedPath(path) => { Self::NamedPath(path) => {
// When used with `git difftool`, the first argument // When used with `git difftool`, the first argument
// is a temporary file that always has the same // is a temporary file that always has the same
// permissions. That doesn't mean the file permissions // permissions. That doesn't mean the file permissions
@ -410,8 +410,8 @@ impl FileArgument {
let metadata = std::fs::metadata(path).ok()?; let metadata = std::fs::metadata(path).ok()?;
Some(metadata.permissions().into()) Some(metadata.permissions().into())
} }
FileArgument::Stdin => None, Self::Stdin => None,
FileArgument::DevNull => None, Self::DevNull => None,
} }
} }
} }
@ -484,11 +484,11 @@ impl FileArgument {
/// argument. /// argument.
pub(crate) fn from_cli_argument(arg: &OsStr) -> Self { pub(crate) fn from_cli_argument(arg: &OsStr) -> Self {
if arg == "/dev/null" { if arg == "/dev/null" {
FileArgument::DevNull Self::DevNull
} else if arg == "-" { } else if arg == "-" {
FileArgument::Stdin Self::Stdin
} else { } else {
FileArgument::NamedPath(PathBuf::from(arg)) Self::NamedPath(PathBuf::from(arg))
} }
} }
@ -497,9 +497,9 @@ impl FileArgument {
pub(crate) fn from_path_argument(arg: &OsStr) -> Self { pub(crate) fn from_path_argument(arg: &OsStr) -> Self {
// For new and deleted files, Git passes `/dev/null` as the reference file. // For new and deleted files, Git passes `/dev/null` as the reference file.
if arg == "/dev/null" { if arg == "/dev/null" {
FileArgument::DevNull Self::DevNull
} else { } else {
FileArgument::NamedPath(PathBuf::from(arg)) Self::NamedPath(PathBuf::from(arg))
} }
} }
} }
@ -507,11 +507,11 @@ impl FileArgument {
impl Display for FileArgument { impl Display for FileArgument {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self { match self {
FileArgument::NamedPath(path) => { Self::NamedPath(path) => {
write!(f, "{}", relative_to_current(path).display()) write!(f, "{}", relative_to_current(path).display())
} }
FileArgument::Stdin => write!(f, "(stdin)"), Self::Stdin => write!(f, "(stdin)"),
FileArgument::DevNull => write!(f, "/dev/null"), Self::DevNull => write!(f, "/dev/null"),
} }
} }
} }

@ -525,54 +525,51 @@ fn from_emacs_mode_header(src: &str) -> Option<Language> {
(Some(cap), _) | (_, Some(cap)) => cap[1].into(), (Some(cap), _) | (_, Some(cap)) => cap[1].into(),
_ => "".into(), _ => "".into(),
}; };
let lang = match mode_name.to_ascii_lowercase().trim() { return Some(match mode_name.to_ascii_lowercase().trim() {
"ada" => Some(Ada), "ada" => Ada,
"c" => Some(C), "c" => C,
"clojure" => Some(Clojure), "clojure" => Clojure,
"csharp" => Some(CSharp), "csharp" => CSharp,
"css" => Some(Css), "css" => Css,
"dart" => Some(Dart), "dart" => Dart,
"c++" => Some(CPlusPlus), "c++" => CPlusPlus,
"elixir" => Some(Elixir), "elixir" => Elixir,
"elm" => Some(Elm), "elm" => Elm,
"elvish" => Some(Elvish), "elvish" => Elvish,
"emacs-lisp" => Some(EmacsLisp), "emacs-lisp" => EmacsLisp,
"fsharp" => Some(FSharp), "fsharp" => FSharp,
"gleam" => Some(Gleam), "gleam" => Gleam,
"go" => Some(Go), "go" => Go,
"haskell" => Some(Haskell), "haskell" => Haskell,
"hcl" => Some(Hcl), "hcl" => Hcl,
"html" => Some(Html), "html" => Html,
"janet" => Some(Janet), "janet" => Janet,
"java" => Some(Java), "java" => Java,
"js" | "js2" => Some(JavaScript), "js" | "js2" => JavaScript,
"lisp" => Some(CommonLisp), "lisp" => CommonLisp,
"nxml" => Some(Xml), "nxml" => Xml,
"objc" => Some(ObjC), "objc" => ObjC,
"perl" => Some(Perl), "perl" => Perl,
"python" => Some(Python), "python" => Python,
"racket" => Some(Racket), "racket" => Racket,
"rjsx" => Some(JavascriptJsx), "rjsx" => JavascriptJsx,
"ruby" => Some(Ruby), "ruby" => Ruby,
"rust" => Some(Rust), "rust" => Rust,
"scala" => Some(Scala), "scala" => Scala,
"scss" => Some(Scss), "scss" => Scss,
"sh" => Some(Bash), "sh" => Bash,
"solidity" => Some(Solidity), "solidity" => Solidity,
"sql" => Some(Sql), "sql" => Sql,
"swift" => Some(Swift), "swift" => Swift,
"toml" => Some(Toml), "toml" => Toml,
"tuareg" => Some(OCaml), "tuareg" => OCaml,
"typescript" => Some(TypeScript), "typescript" => TypeScript,
"verilog" => Some(Verilog), "verilog" => Verilog,
"vhdl" => Some(Vhdl), "vhdl" => Vhdl,
"yaml" => Some(Yaml), "yaml" => Yaml,
"zig" => Some(Zig), "zig" => Zig,
_ => None, _ => continue,
}; });
if lang.is_some() {
return lang;
}
} }
None None

@ -196,13 +196,13 @@ impl<'a> fmt::Debug for Syntax<'a> {
impl<'a> Syntax<'a> { impl<'a> Syntax<'a> {
pub(crate) fn new_list( pub(crate) fn new_list(
arena: &'a Arena<Syntax<'a>>, arena: &'a Arena<Self>,
open_content: &str, open_content: &str,
open_position: Vec<SingleLineSpan>, open_position: Vec<SingleLineSpan>,
children: Vec<&'a Syntax<'a>>, children: Vec<&'a Self>,
close_content: &str, close_content: &str,
close_position: Vec<SingleLineSpan>, close_position: Vec<SingleLineSpan>,
) -> &'a Syntax<'a> { ) -> &'a Self {
// Skip empty atoms: they aren't displayed, so there's no // Skip empty atoms: they aren't displayed, so there's no
// point making our syntax tree bigger. These occur when we're // point making our syntax tree bigger. These occur when we're
// parsing incomplete or malformed programs. // parsing incomplete or malformed programs.
@ -249,11 +249,11 @@ impl<'a> Syntax<'a> {
} }
pub(crate) fn new_atom( pub(crate) fn new_atom(
arena: &'a Arena<Syntax<'a>>, arena: &'a Arena<Self>,
mut position: Vec<SingleLineSpan>, mut position: Vec<SingleLineSpan>,
mut content: String, mut content: String,
kind: AtomKind, kind: AtomKind,
) -> &'a Syntax<'a> { ) -> &'a Self {
// If a parser hasn't cleaned up \r on CRLF files with // If a parser hasn't cleaned up \r on CRLF files with
// comments, discard it. // comments, discard it.
if content.ends_with('\r') { if content.ends_with('\r') {
@ -283,11 +283,11 @@ impl<'a> Syntax<'a> {
} }
} }
pub(crate) fn parent(&self) -> Option<&'a Syntax<'a>> { pub(crate) fn parent(&self) -> Option<&'a Self> {
self.info().parent.get() self.info().parent.get()
} }
pub(crate) fn next_sibling(&self) -> Option<&'a Syntax<'a>> { pub(crate) fn next_sibling(&self) -> Option<&'a Self> {
self.info().next_sibling.get() self.info().next_sibling.get()
} }
@ -681,9 +681,7 @@ impl MatchKind {
pub(crate) fn is_novel(&self) -> bool { pub(crate) fn is_novel(&self) -> bool {
matches!( matches!(
self, self,
MatchKind::Novel { .. } Self::Novel { .. } | Self::NovelWord { .. } | Self::UnchangedPartOfNovelItem { .. }
| MatchKind::NovelWord { .. }
| MatchKind::UnchangedPartOfNovelItem { .. }
) )
} }
} }

@ -27,10 +27,10 @@ pub(crate) enum FileFormat {
impl Display for FileFormat { impl Display for FileFormat {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self { match self {
FileFormat::SupportedLanguage(language) => write!(f, "{}", language_name(*language)), Self::SupportedLanguage(language) => write!(f, "{}", language_name(*language)),
FileFormat::PlainText => write!(f, "Text"), Self::PlainText => write!(f, "Text"),
FileFormat::TextFallback { reason } => write!(f, "Text ({})", reason), Self::TextFallback { reason } => write!(f, "Text ({})", reason),
FileFormat::Binary => write!(f, "Binary"), Self::Binary => write!(f, "Binary"),
} }
} }
} }