Skip to content

Commit

Permalink
fix: Handle multi-line loggers better
Browse files Browse the repository at this point in the history
  • Loading branch information
@sondrelg
sondrelg committed Apr 22, 2023
1 parent bfcd111 commit 292bc92
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
33 changes: 18 additions & 15 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,16 @@ fn fix_content(content: &str, filename: &str) -> (Vec<String>, bool) {
change.new_string_variables.join(", ")
);

// If the logger starts and end on the same line, then we can just replace the old line with the new one
if change.lineno == change.end_lineno {
vec_content[change.lineno - 1 - popped_rows]
.replace_range(&change.col_offset..&change.end_col_offset, &new_logger);
} else {
// Replace things on the first line
let range = change.lineno - popped_rows..change.end_lineno - popped_rows;

vec_content[change.lineno - 1 - popped_rows]
.replace_range(&change.col_offset.., &new_logger);

// Replace things on the last line
vec_content[change.end_lineno - 1 - popped_rows]
.replace_range(..change.end_col_offset, "");

// Delete any in-between rows since these will now be empty,
// after inlining syntax on the first line
vec_content.drain(change.lineno..change.end_lineno - 1);
vec_content.drain(range);
popped_rows += change.end_lineno - change.lineno;
}
}
Expand Down Expand Up @@ -178,7 +173,7 @@ mod tests {
TestCase { input: "foo=1\nlogger.error('{}'.format(foo))".to_string(), expected_output: "foo=1\nlogger.error('%s', foo)".to_string() },
TestCase { input: "foo=1\nlogger.error('{foo}'.format(foo=foo))".to_string(), expected_output: "foo=1\nlogger.error('%s', foo)".to_string() },
// Multi-line
TestCase { input: "logger.error(\n\t'{}'.format(\n\t\t1\n\t)\n)".to_string(), expected_output: "logger.error(\n\t'%s', 1\n\n)".to_string() },
TestCase { input: "logger.error(\n\t'{}'.format(\n\t\t1\n\t)\n)".to_string(), expected_output: "logger.error(\n\t'%s', 1\n)".to_string() },
// Contained by class
TestCase { input: "class Foo:\n\tdef bar(self):\n\t\tlogger.error('{}'.format(1))\n".to_string(), expected_output: "class Foo:\n\tdef bar(self):\n\t\tlogger.error('%s', 1)\n".to_string() },
// Nested properties
Expand All @@ -193,7 +188,6 @@ mod tests {
TestCase { input: "logging.error('Error parsing event file: {}'.format(e.errors()))".to_string(), expected_output: "logging.error('Error parsing event file: %s', e.errors())".to_string() },
// Index
TestCase { input: "logger.error('{}'.format(ret[\"id\"]))".to_string(), expected_output: "logger.error('%s', ret['id'])".to_string() },

]
}

Expand All @@ -213,12 +207,21 @@ mod tests {

#[test]
fn test_fix_content_format_with_too_many_arguments_panics() {
SETTINGS.get_or_init(|| Opts {
log_level: LogLevel::Error,
filenames: vec![],
quotes: Quotes::Single,
});
assert_panic!(
tokio_test::block_on(async {
fix_content("logger.error('{}'.format(1,2))", "filename");
FILENAME
.scope("test".to_string(), async move {
fix_content("logger.error('{}'.format(1,2))", "filename");
})
.await;
}),
String,
"Found excess argument `2` in logger. Run with RUST_LOG=debug for verbose logging.",
"File `test` contains a str.format call with too many arguments for the string. Argument is `2`. Please fix before proceeding.",
);
}

Expand Down Expand Up @@ -247,7 +250,7 @@ mod tests {
// Newline character
TestCase { input: "logger.error(f'{foo}\\n{bar}')".to_string(), expected_output: "logger.error('%s\n%s', foo, bar)".to_string() },
// Multi-line
TestCase { input: "logger.error(\n\tf'foo {bar} '\n\tf'baz %s',\n\te,\n\texc_info=True)".to_string(), expected_output: "logger.error(\n\t'foo %s baz %s', bar\n,\n\te,\n\texc_info=True)".to_string() },
TestCase { input: "logger.error(\n\tf'foo {bar} '\n\tf'baz %s',\n\te,\n\texc_info=True\n)".to_string(), expected_output: "logger.error(\n\t'foo %s baz %s', bar\n\te,\n\texc_info=True\n)".to_string() },
// Call inside f-string
TestCase { input: "logging.error(f'Error parsing event file: {e.errors()}')".to_string(), expected_output: "logging.error('Error parsing event file: %s', e.errors())".to_string() },
// Index inside f-string
Expand Down Expand Up @@ -281,7 +284,7 @@ mod tests {
// Leading argument is not string -- expect no change
TestCase { input: "messages.error(self.request, '{}'.format(foo))".to_string(), expected_output: "messages.error(self.request, '{}'.format(foo))".to_string() },
// Line trim
TestCase { input: "logger.error(\n\tf'{1}'\n\tf'{2}',\n\texc_info=True\n)".to_string(), expected_output: "logger.error(\n\t'%s%s', 1, 2\n,\n\texc_info=True\n)".to_string() },
TestCase { input: "logger.error(\n\tf'{1}'\n\tf'{2}',\n\texc_info=True\n)".to_string(), expected_output: "logger.error(\n\t'%s%s', 1, 2\n\texc_info=True\n)".to_string() },
TestCase { input: "logger.exception(f'foo {bar}')".to_string(), expected_output: "logger.exception('foo %s', bar)".to_string() },
]
}
Expand Down
5 changes: 3 additions & 2 deletions src/parse_format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub fn get_args_and_keywords(
ExprKind::Name { id, .. } => f_args.push(id.to_string()),
_ => {
let filename = FILENAME.with(std::clone::Clone::clone);
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new :)", filename, value.location.row());
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new", filename, value.location.row());
eprintln!("{error_message}");
bail!("");
}
Expand Down Expand Up @@ -134,7 +134,8 @@ fn order_arguments(
// where there are more arguments passed than mapped to.
// We could ignore these cases, but if we silently fixed them
// that might cause other problems for the user ¯\_(ツ)_/¯
panic!("Found excess argument `{arg}` in logger. Run with RUST_LOG=debug for verbose logging.")
let filename = FILENAME.with(std::clone::Clone::clone);
panic!("File `{filename}` contains a str.format call with too many arguments for the string. Argument is `{arg}`. Please fix before proceeding.")
};
let start = mat.start();
let end = mat.end();
Expand Down
6 changes: 3 additions & 3 deletions src/parse_fstring.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub fn parse_formatted_value(value: &Expr, postfix: String) -> Result<String> {
}
_ => {
let filename = FILENAME.with(std::clone::Clone::clone);
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new :)", filename, func.location.row());
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new", filename, func.location.row());
eprintln!("{error_message}");
bail!("")
}
Expand All @@ -95,7 +95,7 @@ pub fn parse_formatted_value(value: &Expr, postfix: String) -> Result<String> {
}
_ => {
let filename = FILENAME.with(std::clone::Clone::clone);
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new :)", filename, value.location.row());
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new", filename, value.location.row());
eprintln!("{error_message}");
bail!("");
}
Expand All @@ -120,7 +120,7 @@ fn parse_fstring(value: &Expr, string: &mut String, args: &mut Vec<String>) -> R
}
_ => {
let filename = FILENAME.with(std::clone::Clone::clone);
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new :)", filename, value.location.row());
let error_message = format!("Failed to parse `{}` line {}. Please open an issue at https://github.com/sondrelg/printf-log-formatter/issues/new", filename, value.location.row());
eprintln!("{error_message}");
bail!("");
}
Expand Down

0 comments on commit 292bc92

Please sign in to comment.