rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

Snippet rework

Open Jarcho opened this issue 8 months ago • 16 comments

This is a further rework of our snippet accessing/creating code.

The general design goals with this:

  • Avoid allocating when not strictly necessary.
  • Make it easy to avoid linting if an invalid span is used.
  • Make it easier to debug when an invalid span is used.
  • Make it easy to minimize the number of lookups in the source map.
  • Make it easy to avoid creating intermediary compressed spans.
  • Make it as easy as possible to compose span adjustments/checks while maintaining the previous goals.

changelog: None

Jarcho avatar May 02 '25 05:05 Jarcho

r? @Alexendoo

rustbot has assigned @Alexendoo. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot avatar May 02 '25 05:05 rustbot

The first two commits are mainly renames. The third commit has all the actual changes.

Jarcho avatar May 02 '25 05:05 Jarcho

Looking at MISSING_DOCS_IN_PRIVATE_ITEMS, that lint needs to be restructured a bit. Macros currently mess up it's implementation quite a bit.

Why is this not part of rustc in the first place?

Jarcho avatar May 02 '25 05:05 Jarcho

:umbrella: The latest upstream changes (possibly 56f018286b8feeed22284afc14c97aa2600afb88) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar May 02 '25 17:05 rustbot

:warning: Warning :warning:

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    • 1c301634a3615050a94e90d34a8307d7d0dac2c1

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust-clippy.git master
    $ git push --force-with-lease
    

rustbot avatar Jul 18 '25 15:07 rustbot

No changes for 1eddd7563355561f301176b87c0741700766d14c

github-actions[bot] avatar Jul 18 '25 15:07 github-actions[bot]

Should be ready, but still waiting on #14741 to be merged. Recent changes:

  • Debug assert message now has a lot more info. The amount of info added is quite helpful when debugging span issues. SourceMap has to be threaded through when debug assertions are enabled. to make the cross file notes.
  • #[track_caller] is used to get the panic location to give a useful position. This is way easier then reading the stack trace.
  • Setting the start and end are split into if_within and if_before/after versions. The caller should always know which one should happen, but macros can cause tokens to get rearranged in fun ways.

I'm still very much against trying to merge SourceText and SourceFileRange. They model notably different things.

Jarcho avatar Jul 20 '25 15:07 Jarcho

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rustbot avatar Sep 25 '25 03:09 rustbot

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rustbot avatar Nov 08 '25 20:11 rustbot

:umbrella: The latest upstream changes (possibly d5995292239bf70760cb63bac61ca7b5e8fd1d4d) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar Nov 11 '25 10:11 rustbot

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rustbot avatar Nov 16 '25 23:11 rustbot

The API has changed again; this time to support splitting a range. Changes include:

  • get_source_text has been renamed to get_text. The "source" part of the name doesn't really add anything and just makes it longer. check_source_text was renamed similarly.
  • SourceFileRange has been replaced with SpanEditCx which no longer contains the range.
  • map_range's callback now takes both the edit context and the range as separate parameters and returns either a single range or an array of ranges.
  • Functions for shrinking a range based on it's contents have been replaced by a general &str -> &str transformation that recalculates the range positions. This can also return an array of strings.
  • Some entry points no longer load external sources. get_text and check_text are still loading external sources since they hit debug assertions otherwise.
  • A couple of general utils have been added (display and StrExt). They aren't specific to touching the source code, but they will mainly be used when working with it.

I might switch the range editing functions to be on the range, but that means an extra trait will need to be imported everywhere. range.with_leading_prefix(scx, "prefix") is nicer when chaining multiple, so it's probably worth it.

Jarcho avatar Nov 17 '25 02:11 Jarcho

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rustbot avatar Nov 18 '25 04:11 rustbot

:umbrella: The latest upstream changes (possibly 4292841759d642a014ec4ac5fb779d51ee10a51a) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar Nov 26 '25 20:11 rustbot

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rustbot avatar Dec 11 '25 10:12 rustbot

:umbrella: The latest upstream changes (possibly 9e3e9649cbae7ed33ba62ca436f43081eaaedeb5) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar Dec 11 '25 18:12 rustbot