notes · · 6 min

The errors were there before the edit

A new refactoring made a field private and rewrote every access to it, and its diff was right. Its verdict was 124 errors. None of them was the edit's: the file had them before anyone touched it, and every write tool had been counting them.

On this page · 6 sections
  1. Nobody had touched the file
  2. What was already there
  3. The refactoring itself
  4. What the analyzer does not check
  5. What it costs
  6. What it does not do

The first real run of the new tool was on ExecRequest.timeout_secs, a public field of a message struct that two other crates use. It was asked to make the field private and turn every access to it outside its file into a method call. The diff was right:

-    let timeout = std::time::Duration::from_secs(if req.timeout_secs == 0 {
+    let timeout = std::time::Duration::from_secs(if req.timeout_secs() == 0 {
         EXEC_DEFAULT_TIMEOUT_SECS
     } else {
-        req.timeout_secs
+        req.timeout_secs()
     });

The verdict under it was not:

the analyzer rejects the result:
  type annotations needed [E0282] (crates/prod-code-protocol/src/messages.rs:6:3)
  type annotations needed [E0282] (crates/prod-code-protocol/src/messages.rs:6:3)
  type annotations needed [E0282] (crates/prod-code-protocol/src/messages.rs:57:3)
  …

One hundred and twenty-four lines of it, every one on a #[derive(…, Deserialize)]. With that verdict, apply refuses to write.

Nobody had touched the file

The obvious reading is that the edit caused them: the field changed, and code generated from the struct no longer type-checks. One command says otherwise. Ask the same check about the file with nothing changed at all:

$ prod-code validate crates/prod-code-protocol/src/messages.rs --from crates/prod-code-protocol/src/messages.rs
crates/prod-code-protocol/src/messages.rs: 124 error(s), 0 warning(s)

All 124 were there before the edit. The analyzer expands serde’s derives - give a struct a field of a type that does not exist and it says the trait bound `!: Deserialize<'_>` is not satisfied at that field, which only the generated code can say - and then fails to infer types inside what Deserialize generates. It does that in every file that has such a derive, edit or no edit: 50 errors in another file of this workspace, 26 in a third, 19 in a fourth. None of it is real - the same files compile.

That is the whole problem, and it had been hiding in plain sight. The check that runs before every write - the one this series introduced as the analyzer already knowing your patch is broken - reports what the analyzer says about the proposed text. What the analyzer says about the proposed text includes everything it already said about the old one. So any refactoring that touched one of these files ended with “the analyzer rejects the result”, and a tool that refuses on errors refused. The post on type migration had met the same 124 and, at first, explained them as a side effect of the change; that explanation did not survive this command, and the post now says so.

What was already there

The fix is the question a person asks without thinking about it: was this here before I started? Before any proposed text is opened, each file’s diagnostics are pulled as it is on disk (#79). A diagnostic in the result that the file already had is set aside and counted, not blamed on the edit.

“Already had” needs a definition, and the obvious one is wrong. Positions move - an edit that adds a line above an old error moves the error down one - so a diagnostic is the same one when it has the same severity, code and message on a line with the same text. And each old diagnostic accounts for at most one new one. That matters more than it looks. Give two of those structs a field of a type that does not exist, and besides an error at each new field, a derive further down the file reports two more copies of an error it already had. Those two are the edit’s, and the check still says so:

crates/prod-code-protocol/src/messages.rs: 4 error(s), 0 warning(s)
  (124 diagnostic(s) the file already had before this edit are not counted: 120× type annotations needed [E0282], 4× type annotations needed; type must be known at this point [E0282])

An edit that changes nothing now gets 0 error(s), with the 124 named in one line instead of listed in 124.

The refactoring itself

rust-analyzer can generate a getter and a setter for a field and make the field private, one assist at a time. The part it leaves to you is the part that takes the afternoon: every x.field in the rest of the workspace no longer compiles, and each one has to become x.field() or x.set_field(v) by hand, one build error at a time. code_encapsulate_field does that part.

On the left, the declaring file: the field loses pub, and a getter and a setter are added; accesses in this file stay direct because a private field is visible there. On the right, every other file: a read becomes a getter call, a plain assignment becomes a setter call, and a struct literal, a compound assignment and a mutable borrow are reported with their line, and nothing is written. One overlay type-checks all of it before anything is written
Inside the declaring file only the declaration and the new accessors change. Outside it, every access becomes a call or a reason not to write.

The analyzer finds every reference to the field. What each one does is read from the text around it:

at the reference what it becomes
cfg.retries cfg.retries()
cfg.retries = n cfg.set_retries(n)
cfg.retries += 1 reported: it needs the getter and the setter
&mut cfg.retries reported: a private field cannot be borrowed from outside
Config { retries: 5 }, let Config { retries, .. } reported: a literal or pattern cannot name a private field

References inside the declaring file are left alone, because a private field is still visible there. A use from the last three rows blocks the write: making the field private would break it, and there is no method call to put in its place. And a position that does not hold the field’s name is reported and not touched - the check the previous post added to the tools that edit at the analyzer’s positions.

The getter returns the value for a primitive Copy type and a shared reference for anything else. Returning a String by value would move it out; returning a u32 by reference would make every cfg.retries() == 0 a type error. The setter is generated only when something outside the file writes the field. Both go into the struct’s own impl, with the visibility the field had.

On ExtractedParameter.applied, which the tool that reports on it sets from another file:

`ExtractedParameter.applied` (crates/prod-code-mcp/src/extract_parameter.rs)

- the field becomes private
- getter: `fn applied(&self) -> bool`
- setter: `fn set_applied(&mut self, applied: bool)`
- 2 read(s) and 1 write(s) outside crates/prod-code-mcp/src/extract_parameter.rs rewritten; 3 reference(s) inside it left as they are, because a private field is still visible there

-                done.applied = true;
+                done.set_applied(true);
…
-    assert!(!done.applied);
+    assert!(!done.applied());
…

the analyzer accepts the result: 0 errors

the compiler accepts the result too: `cargo check` in a shadow of the workspace, 3312 ms

And on timeout_secs, with the 124 out of the way, the one thing that really stands in the way:

1 use(s) outside the declaring file cannot become a method call, and a private field would not compile there:
  crates/prod-code-mcp/src/exec.rs:68:13 a struct literal or pattern names the field: `timeout_secs,`

the analyzer accepts the result: 0 errors

What the analyzer does not check

A getter that returns &String is fine for c.name.len() and wrong for c.name.push('!'), and the difference is the borrow checker’s - which the analyzer does not run. So when a read goes on to call a method on a field returned by reference, the report says so and points at verify: "compile", which runs cargo check in a shadow of the workspace before anything is written. That is the only check that sees it.

What it costs

More than it should. A dry run on ExtractedParameter.applied took 49.5, 46.0 and 44.2 seconds. The tool’s own work is a fraction of a second: the check on the changed file alone takes 0.48 s. The rest is the cost #73 already measured. Making a field private changes what a file declares, the analyzer re-resolves names for the crate that imports it, and the next query pays:

refs, settled                                0.10 s
the field made private, in an overlay        0.48 s
refs, right after                           21.8 s

A dry run pays it about twice - 44 to 50 seconds against 21.8 - once, it appears, for the previous run’s overlay closing and once for its own. That is the next thing to fix.

What it does not do

  • Named fields only, and the accessors go into an impl in the declaring file. A generic struct with no impl there is refused, rather than guessing its bounds.
  • No &mut getter. A &mut cfg.field outside the file is a reason to stop, not something to rewrite into cfg.field_mut().
  • One field at a time, and the field must be public: a private one has nothing outside its module to rewrite.
  • Rust only.

The discovery was bigger than the tool. A check that is right about the edit and wrong about the file blocks exactly the refactorings that touch the most-used code, where they matter most - and it is invisible until something asks the plain question of what was there before.

Cite this article
Citation
Alexander Panasenko (2026-10-08). The errors were there before the edit. https://prod.codes/blog/the-errors-were-there-before-the-edit/