Skip to content

Commit

Permalink
Fix #2104: fmt incorrectly using 1-indexing for columns (#2106)
Browse files Browse the repository at this point in the history
* Fix #2104: fmt incorrectly using 1-indexing for columns

* Clippy...
  • Loading branch information
jkelleyrtp authored Mar 19, 2024
1 parent 2dc6cec commit d8942a2
Show file tree
Hide file tree
Showing 17 changed files with 756 additions and 40 deletions.
14 changes: 2 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 2 additions & 8 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,10 @@ rustc-hash = "1.1.0"
wasm-bindgen = "0.2.92"
html_parser = "0.7.0"
thiserror = "1.0.40"
prettyplease = { package = "prettier-please", version = "0.2", features = [
"verbatim",
] }
manganis-cli-support = { version = "0.2.1", features = [
"html",
] }
prettyplease = { version = "0.2.16", features = ["verbatim"] }
manganis-cli-support = { version = "0.2.1", features = ["html"] }
manganis = { version = "0.2.1" }

interprocess = { version = "1.2.1" }
# interprocess = { git = "https://github.com/kotauskas/interprocess" }

lru = "0.12.2"
async-trait = "0.1.77"
Expand Down
12 changes: 6 additions & 6 deletions packages/autofmt/src/component.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{ifmt_to_string, writer::Location, Writer};
use crate::{ifmt_to_string, prettier_please::unparse_expr, writer::Location, Writer};
use dioxus_rsx::*;
use quote::ToTokens;
use std::fmt::{Result, Write};
Expand Down Expand Up @@ -164,7 +164,7 @@ impl Writer<'_> {
let name = &field.name;
match &field.content {
ContentField::ManExpr(exp) => {
let out = prettyplease::unparse_expr(exp);
let out = unparse_expr(exp);
let mut lines = out.split('\n').peekable();
let first = lines.next().unwrap();
write!(self.out, "{name}: {first}")?;
Expand All @@ -186,7 +186,7 @@ impl Writer<'_> {
write!(self.out, "{}", e.to_token_stream())?;
}
ContentField::OnHandlerRaw(exp) => {
let out = prettyplease::unparse_expr(exp);
let out = unparse_expr(exp);
let mut lines = out.split('\n').peekable();
let first = lines.next().unwrap();
write!(self.out, "{name}: {first}")?;
Expand Down Expand Up @@ -228,7 +228,7 @@ impl Writer<'_> {
ContentField::Formatted(s) => ifmt_to_string(s).len() ,
ContentField::Shorthand(e) => e.to_token_stream().to_string().len(),
ContentField::OnHandlerRaw(exp) | ContentField::ManExpr(exp) => {
let formatted = prettyplease::unparse_expr(exp);
let formatted = unparse_expr(exp);
let len = if formatted.contains('\n') {
10000
} else {
Expand All @@ -242,7 +242,7 @@ impl Writer<'_> {

match manual_props {
Some(p) => {
let content = prettyplease::unparse_expr(p);
let content = unparse_expr(p);
if content.len() + attr_len > 80 {
return 100000;
}
Expand All @@ -264,7 +264,7 @@ impl Writer<'_> {
We want to normalize the expr to the appropriate indent level.
*/

let formatted = prettyplease::unparse_expr(exp);
let formatted = unparse_expr(exp);

let mut lines = formatted.lines();

Expand Down
19 changes: 11 additions & 8 deletions packages/autofmt/src/element.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{ifmt_to_string, Writer};
use crate::{ifmt_to_string, prettier_please::unparse_expr, Writer};
use dioxus_rsx::*;
use proc_macro2::Span;
use quote::ToTokens;
Expand Down Expand Up @@ -112,7 +112,7 @@ impl Writer<'_> {
ShortOptimization::Oneliner => {
write!(self.out, " ")?;

self.write_attributes(attributes, key, true)?;
self.write_attributes(brace, attributes, key, true)?;

if !children.is_empty() && (!attributes.is_empty() || key.is_some()) {
write!(self.out, ", ")?;
Expand All @@ -132,7 +132,7 @@ impl Writer<'_> {
if !attributes.is_empty() || key.is_some() {
write!(self.out, " ")?;
}
self.write_attributes(attributes, key, true)?;
self.write_attributes(brace, attributes, key, true)?;

if !children.is_empty() && (!attributes.is_empty() || key.is_some()) {
write!(self.out, ",")?;
Expand All @@ -145,7 +145,7 @@ impl Writer<'_> {
}

ShortOptimization::NoOpt => {
self.write_attributes(attributes, key, false)?;
self.write_attributes(brace, attributes, key, false)?;

if !children.is_empty() && (!attributes.is_empty() || key.is_some()) {
write!(self.out, ",")?;
Expand All @@ -166,6 +166,7 @@ impl Writer<'_> {

fn write_attributes(
&mut self,
brace: &Brace,
attributes: &[AttributeType],
key: &Option<IfmtInput>,
sameline: bool,
Expand All @@ -187,9 +188,11 @@ impl Writer<'_> {

while let Some(attr) = attr_iter.next() {
self.out.indent_level += 1;

if !sameline {
self.write_comments(attr.start())?;
self.write_attr_comments(brace, attr.start())?;
}

self.out.indent_level -= 1;

if !sameline {
Expand Down Expand Up @@ -229,7 +232,7 @@ impl Writer<'_> {
write!(
self.out,
"if {condition} {{ ",
condition = prettyplease::unparse_expr(condition),
condition = unparse_expr(condition),
)?;
self.write_attribute_value(value)?;
write!(self.out, " }}")?;
Expand All @@ -241,7 +244,7 @@ impl Writer<'_> {
write!(self.out, "{value}",)?;
}
ElementAttrValue::AttrExpr(value) => {
let out = prettyplease::unparse_expr(value);
let out = unparse_expr(value);
let mut lines = out.split('\n').peekable();
let first = lines.next().unwrap();

Expand Down Expand Up @@ -308,7 +311,7 @@ impl Writer<'_> {

fn write_spread_attribute(&mut self, attr: &Expr) -> Result {
write!(self.out, "..")?;
write!(self.out, "{}", prettyplease::unparse_expr(attr))?;
write!(self.out, "{}", unparse_expr(attr))?;

Ok(())
}
Expand Down
3 changes: 2 additions & 1 deletion packages/autofmt/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ impl Writer<'_> {
// If the expr is multiline, we want to collect all of its lines together and write them out properly
// This involves unshifting the first line if it's aligned
let first_line = &self.src[start.line - 1];
write!(self.out, "{}", &first_line[start.column - 1..].trim_start())?;
write!(self.out, "{}", &first_line[start.column..].trim_start())?;

let prev_block_indent_level = self.out.indent.count_indents(first_line);

for (id, line) in self.src[start.line..end.line].iter().enumerate() {
writeln!(self.out)?;

// check if this is the last line
let line = {
if id == (end.line - start.line) - 1 {
Expand Down
1 change: 1 addition & 0 deletions packages/autofmt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod component;
mod element;
mod expr;
mod indent;
mod prettier_please;
mod writer;

pub use indent::{IndentOptions, IndentType};
Expand Down
66 changes: 66 additions & 0 deletions packages/autofmt/src/prettier_please.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use prettyplease::unparse;
use syn::{Expr, File, Item};

/// Unparse an expression back into a string
///
/// This creates a new temporary file, parses the expression into it, and then formats the file.
/// This is a bit of a hack, but dtonlay doesn't want to support this very simple usecase, forcing us to clone the expr
pub fn unparse_expr(expr: &Expr) -> String {
let file = wrapped(expr);
let wrapped = unparse(&file);
unwrapped(wrapped)
}

// Split off the fn main and then cut the tabs off the front
fn unwrapped(raw: String) -> String {
raw.strip_prefix("fn main() {\n")
.unwrap()
.strip_suffix("}\n")
.unwrap()
.lines()
.map(|line| line.strip_prefix(" ").unwrap()) // todo: set this to tab level
.collect::<Vec<_>>()
.join("\n")
}

fn wrapped(expr: &Expr) -> File {
File {
shebang: None,
attrs: vec![],
items: vec![
//
Item::Verbatim(quote::quote! {
fn main() {
#expr
}
}),
],
}
}

#[test]
fn unparses_raw() {
let expr = syn::parse_str("1 + 1").unwrap();
let unparsed = unparse(&wrapped(&expr));
assert_eq!(unparsed, "fn main() {\n 1 + 1\n}\n");
}

#[test]
fn unparses_completely() {
let expr = syn::parse_str("1 + 1").unwrap();
let unparsed = unparse_expr(&expr);
assert_eq!(unparsed, "1 + 1");
}

#[test]
fn weird_ifcase() {
let contents = r##"
fn main() {
move |_| timer.with_mut(|t| if t.started_at.is_none() { Some(Instant::now()) } else { None })
}
"##;

let expr: File = syn::parse_file(contents).unwrap();
let out = unparse(&expr);
println!("{}", out);
}
29 changes: 24 additions & 5 deletions packages/autofmt/src/writer.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use crate::prettier_please::unparse_expr;
use dioxus_rsx::{AttributeType, BodyNode, ElementAttrValue, ForLoop, IfChain};
use proc_macro2::{LineColumn, Span};
use quote::ToTokens;
use std::{
collections::{HashMap, VecDeque},
fmt::{Result, Write},
};
use syn::{spanned::Spanned, Expr};
use syn::{spanned::Spanned, token::Brace, Expr};

use crate::buffer::Buffer;
use crate::ifmt_to_string;
Expand Down Expand Up @@ -61,8 +62,26 @@ impl<'a> Writer<'a> {
Some(self.out.buf)
}

pub fn write_attr_comments(&mut self, brace: &Brace, attr_span: Span) -> Result {
// There's a chance this line actually shares the same line as the previous
// Only write comments if the comments actually belong to this line
//
// to do this, we check if the attr span starts on the same line as the brace
// if it doesn't, we write the comments
let brace_line = brace.span.span().start().line;
let attr_line = attr_span.start().line;

if brace_line != attr_line {
self.write_comments(attr_span)?;
}

Ok(())
}

pub fn write_comments(&mut self, child: Span) -> Result {
// collect all comments upwards
// make sure we don't collect the comments of the node that we're currently under.

let start = child.start();
let line_start = start.line - 1;

Expand Down Expand Up @@ -149,7 +168,7 @@ impl<'a> Writer<'a> {
let len = if let std::collections::hash_map::Entry::Vacant(e) =
self.cached_formats.entry(location)
{
let formatted = prettyplease::unparse_expr(tokens);
let formatted = unparse_expr(tokens);
let len = if formatted.contains('\n') {
10000
} else {
Expand Down Expand Up @@ -207,7 +226,7 @@ impl<'a> Writer<'a> {
pub fn retrieve_formatted_expr(&mut self, expr: &Expr) -> &str {
self.cached_formats
.entry(Location::new(expr.span().start()))
.or_insert_with(|| prettyplease::unparse_expr(expr))
.or_insert_with(|| unparse_expr(expr))
.as_str()
}

Expand All @@ -216,7 +235,7 @@ impl<'a> Writer<'a> {
self.out,
"for {} in {} {{",
forloop.pat.clone().into_token_stream(),
prettyplease::unparse_expr(&forloop.expr)
unparse_expr(&forloop.expr)
)?;

if forloop.body.is_empty() {
Expand Down Expand Up @@ -249,7 +268,7 @@ impl<'a> Writer<'a> {
self.out,
"{} {} {{",
if_token.to_token_stream(),
prettyplease::unparse_expr(cond)
unparse_expr(cond)
)?;

self.write_body_indented(then_branch)?;
Expand Down
1 change: 1 addition & 0 deletions packages/autofmt/tests/samples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,5 @@ twoway![
tiny,
tinynoopt,
trailing_expr,
many_exprs,
];
Loading

0 comments on commit d8942a2

Please sign in to comment.