Skip to content

Commit 148105a

Browse files
committed
Add rustfix::apply_suggestions_separately
1 parent 06daef6 commit 148105a

16 files changed

+182
-40
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/rustfix/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "rustfix"
3-
version = "0.8.4"
3+
version = "0.8.5"
44
authors = [
55
"Pascal Hertleif <[email protected]>",
66
"Oliver Schneider <[email protected]>",

crates/rustfix/src/lib.rs

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
213213
/// 1. Feeds the source of a file to [`CodeFix::new`].
214214
/// 2. Calls [`CodeFix::apply`] to apply suggestions to the source code.
215215
/// 3. Calls [`CodeFix::finish`] to get the "fixed" code.
216+
#[derive(Clone)]
216217
pub struct CodeFix {
217218
data: replace::Data,
218219
/// Whether or not the data has been modified.
@@ -230,12 +231,18 @@ impl CodeFix {
230231

231232
/// Applies a suggestion to the code.
232233
pub fn apply(&mut self, suggestion: &Suggestion) -> Result<(), Error> {
233-
for sol in &suggestion.solutions {
234-
for r in &sol.replacements {
235-
self.data
236-
.replace_range(r.snippet.range.clone(), r.replacement.as_bytes())?;
237-
self.modified = true;
238-
}
234+
for solution in &suggestion.solutions {
235+
self.apply_solution(solution)?;
236+
}
237+
Ok(())
238+
}
239+
240+
/// Applies an individual solution from a [`Suggestion`]
241+
pub fn apply_solution(&mut self, solution: &Solution) -> Result<(), Error> {
242+
for r in &solution.replacements {
243+
self.data
244+
.replace_range(r.snippet.range.clone(), r.replacement.as_bytes())?;
245+
self.modified = true;
239246
}
240247
Ok(())
241248
}
@@ -252,6 +259,9 @@ impl CodeFix {
252259
}
253260

254261
/// Applies multiple `suggestions` to the given `code`.
262+
///
263+
/// When a suggestion has multiple solutions they are merged together into a
264+
/// single output, to apply them separately see [`apply_suggestions_separately`]
255265
pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> Result<String, Error> {
256266
let mut already_applied = HashSet::new();
257267
let mut fix = CodeFix::new(code);
@@ -270,3 +280,34 @@ pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> Result<Strin
270280
}
271281
fix.finish()
272282
}
283+
284+
/// Applies multiple `suggestions` to the given `code`.
285+
///
286+
/// When a suggestion has multiple solutions that they are applied separately
287+
/// each output being the result of only applying the Nth solution. If a
288+
/// suggestion has fewer solutions than other suggestions the first solution
289+
/// is repeated.
290+
///
291+
/// Currently individual subdiagnostics with multiple solutions are not
292+
/// detected, they are treated as a single combined solution. See
293+
/// [rust-lang/rust#53934](https://github.com/rust-lang/rust/issues/53934).
294+
pub fn apply_suggestions_separately(
295+
code: &str,
296+
suggestions: &[Suggestion],
297+
) -> Result<Vec<String>, Error> {
298+
let max_solutions = suggestions
299+
.iter()
300+
.map(|suggestion| suggestion.solutions.len())
301+
.max()
302+
.unwrap_or_default();
303+
let mut fixes = vec![CodeFix::new(code); max_solutions];
304+
for Suggestion { solutions, .. } in suggestions.iter().rev() {
305+
// If this suggestion does not have as many solutions as needed, repeat
306+
// the first solution
307+
let repeat_first = std::iter::from_fn(|| solutions.first());
308+
for (solution, fix) in solutions.iter().chain(repeat_first).zip(&mut fixes) {
309+
fix.apply_solution(solution)?;
310+
}
311+
}
312+
fixes.iter().map(CodeFix::finish).collect()
313+
}

crates/rustfix/tests/parse_and_replace.rs

Lines changed: 57 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
#![allow(clippy::disallowed_methods, clippy::print_stdout, clippy::print_stderr)]
2424

2525
use anyhow::{anyhow, ensure, Context, Error};
26-
use rustfix::apply_suggestions;
26+
use rustfix::{apply_suggestions, apply_suggestions_separately};
2727
use std::collections::HashSet;
2828
use std::env;
2929
use std::ffi::OsString;
@@ -33,8 +33,12 @@ use std::process::{Command, Output};
3333
use tempfile::tempdir;
3434
use tracing::{debug, info, warn};
3535

36-
mod fixmode {
37-
pub const EVERYTHING: &str = "yolo";
36+
#[derive(Clone, Copy)]
37+
enum ReplaceMode {
38+
/// Combine suggestions into a single `.fixed` file with [`apply_suggestions`]
39+
Combined,
40+
/// Create a separate `.fixed` file per suggestion with [`apply_suggestions_separately`]
41+
Separate,
3842
}
3943

4044
mod settings {
@@ -151,23 +155,16 @@ fn diff(expected: &str, actual: &str) -> String {
151155
res
152156
}
153157

154-
fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Error> {
158+
fn test_rustfix_with_file<P: AsRef<Path>>(file: P, replace_mode: ReplaceMode) -> Result<(), Error> {
155159
let file: &Path = file.as_ref();
156160
let json_file = file.with_extension("json");
157-
let fixed_file = file.with_extension("fixed.rs");
158-
159-
let filter_suggestions = if mode == fixmode::EVERYTHING {
160-
rustfix::Filter::Everything
161-
} else {
162-
rustfix::Filter::MachineApplicableOnly
163-
};
164161

165162
debug!("next up: {:?}", file);
166163
let code = fs::read_to_string(file)?;
167164
let errors =
168165
compile_and_get_json_errors(file).context(format!("could compile {}", file.display()))?;
169166
let suggestions =
170-
rustfix::get_suggestions_from_json(&errors, &HashSet::new(), filter_suggestions)
167+
rustfix::get_suggestions_from_json(&errors, &HashSet::new(), rustfix::Filter::Everything)
171168
.context("could not load suggestions")?;
172169

173170
if std::env::var(settings::RECORD_JSON).is_ok() {
@@ -179,9 +176,12 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
179176
"could not load json fixtures for {}",
180177
file.display()
181178
))?;
182-
let expected_suggestions =
183-
rustfix::get_suggestions_from_json(&expected_json, &HashSet::new(), filter_suggestions)
184-
.context("could not load expected suggestions")?;
179+
let expected_suggestions = rustfix::get_suggestions_from_json(
180+
&expected_json,
181+
&HashSet::new(),
182+
rustfix::Filter::Everything,
183+
)
184+
.context("could not load expected suggestions")?;
185185

186186
ensure!(
187187
expected_suggestions == suggestions,
@@ -193,28 +193,52 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
193193
);
194194
}
195195

196-
let fixed = apply_suggestions(&code, &suggestions)
197-
.context(format!("could not apply suggestions to {}", file.display()))?
198-
.replace('\r', "");
196+
let bless = std::env::var_os(settings::BLESS)
197+
.is_some_and(|bless_name| bless_name == file.file_name().unwrap());
199198

200-
if std::env::var(settings::RECORD_FIXED_RUST).is_ok() {
201-
fs::write(file.with_extension("recorded.rs"), &fixed)?;
199+
if bless {
200+
std::fs::write(&json_file, &errors)?;
202201
}
203202

204-
if let Some(bless_name) = std::env::var_os(settings::BLESS) {
205-
if bless_name == file.file_name().unwrap() {
206-
std::fs::write(&json_file, &errors)?;
207-
std::fs::write(&fixed_file, &fixed)?;
203+
match replace_mode {
204+
ReplaceMode::Combined => {
205+
let fixed = apply_suggestions(&code, &suggestions)
206+
.context(format!("could not apply suggestions to {}", file.display()))?
207+
.replace('\r', "");
208+
check_fixed_file(fixed, file.with_extension("fixed.rs"), bless)?;
208209
}
210+
ReplaceMode::Separate => {
211+
let fixes = apply_suggestions_separately(&code, &suggestions)
212+
.context(format!("could not apply suggestions to {}", file.display()))?;
213+
for (i, fixed) in fixes.iter().enumerate() {
214+
check_fixed_file(
215+
fixed.replace('\r', ""),
216+
file.with_extension(format!("fixed.{i}.rs")),
217+
bless,
218+
)?;
219+
}
220+
}
221+
}
222+
223+
Ok(())
224+
}
225+
226+
fn check_fixed_file(fixed: String, fixed_file: PathBuf, bless: bool) -> Result<(), Error> {
227+
if std::env::var(settings::RECORD_FIXED_RUST).is_ok() {
228+
fs::write(fixed_file.with_extension("recorded.rs"), &fixed)?;
229+
}
230+
231+
if bless {
232+
std::fs::write(&fixed_file, &fixed)?;
209233
}
210234

211235
let expected_fixed = fs::read_to_string(&fixed_file)
212-
.context(format!("could read fixed file for {}", file.display()))?
236+
.context(format!("could read fixed file {}", fixed_file.display()))?
213237
.replace('\r', "");
214238
ensure!(
215239
fixed.trim() == expected_fixed.trim(),
216-
"file {} doesn't look fixed:\n{}",
217-
file.display(),
240+
"file {} doesn't match expected fix:\n{}",
241+
fixed_file.display(),
218242
diff(fixed.trim(), expected_fixed.trim())
219243
);
220244

@@ -229,12 +253,12 @@ fn get_fixture_files(p: &str) -> Result<Vec<PathBuf>, Error> {
229253
.filter(|p| p.is_file())
230254
.filter(|p| {
231255
let x = p.to_string_lossy();
232-
x.ends_with(".rs") && !x.ends_with(".fixed.rs") && !x.ends_with(".recorded.rs")
256+
x.ends_with(".rs") && !x.contains(".fixed.") && !x.ends_with(".recorded.rs")
233257
})
234258
.collect())
235259
}
236260

237-
fn assert_fixtures(dir: &str, mode: &str) {
261+
fn assert_fixtures(dir: &str, replace_mode: ReplaceMode) {
238262
let files = get_fixture_files(dir)
239263
.context(format!("couldn't load dir `{}`", dir))
240264
.unwrap();
@@ -254,7 +278,7 @@ fn assert_fixtures(dir: &str, mode: &str) {
254278
info!("skipped: {file:?}");
255279
continue;
256280
}
257-
if let Err(err) = test_rustfix_with_file(file, mode) {
281+
if let Err(err) = test_rustfix_with_file(file, replace_mode) {
258282
println!("failed: {}", file.display());
259283
warn!("{:?}", err);
260284
failures += 1;
@@ -273,7 +297,8 @@ fn assert_fixtures(dir: &str, mode: &str) {
273297
}
274298

275299
#[test]
276-
fn everything() {
300+
fn fixtures() {
277301
tracing_subscriber::fmt::init();
278-
assert_fixtures("./tests/everything", fixmode::EVERYTHING);
302+
assert_fixtures("./tests/everything", ReplaceMode::Combined);
303+
assert_fixtures("./tests/separate", ReplaceMode::Separate);
279304
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
pub fn f(_: i32) -> &'static i32 {
2+
unimplemented!()
3+
}
4+
5+
fn main() {
6+
let v = vec![String::new()];
7+
let _unused = &v[0];
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
pub fn f(_: &i32) -> &i32 {
2+
unimplemented!()
3+
}
4+
5+
fn main() {
6+
let v = vec![String::new()];
7+
let _unused = v[0].clone();
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
pub fn f(_: i32) -> i32 {
2+
unimplemented!()
3+
}
4+
5+
fn main() {
6+
let v = vec![String::new()];
7+
let _unused = &v[0];
8+
}

0 commit comments

Comments
 (0)