Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(linter): Let fixer functions decide whether to apply a fix or not #4187

Closed
Tracked by #4179
DonIsaac opened this issue Jul 11, 2024 · 0 comments
Closed
Tracked by #4179
Labels
A-linter Area - Linter

Comments

@DonIsaac
Copy link
Contributor

DonIsaac commented Jul 11, 2024

Rules that have fixers for some cases, but not others, all have some logic that looks like this:

fn run<'a>(&self, ctx: &LintContext<'a>) {
  // Check for a violation
  let diagnostic = create_diagnostic_for_rule();
  let can_fix = check_if_fixable(); // <-- this is problematic
  if can_fix {
    ctx.diagnostic_with_fix(diagnostic, |fixer| do_fix(ctx, fixer, node));
  } else {
    ctx.diagnostic(diagnostic);
  }
}

This pushes a lot of fixing logic outside of the fixer. In cases where --fix is not used, these are all wasted cycles. If we move fixability checks to inside the fixer's closure, we can prevent rules from doing work that just gets thrown away.

fn run<'a>(&self, ctx: &LintContext<'a>) {
  // Check for a violation
  let diagnostic = create_diagnostic_for_rule();
  ctx.diagnostic_with_fix(diagnostic, |fixer| {
    if !check_if_fixable(ctx, fixer, node) {
      return None;
    }
    return do_fix(ctx, fixer, node);
  });
}
@DonIsaac DonIsaac added the A-linter Area - Linter label Jul 11, 2024
@DonIsaac DonIsaac changed the title feat(linter): Let fixer functions decide whether to apply a fix or not refactor(linter): Let fixer functions decide whether to apply a fix or not Jul 11, 2024
mysteryven pushed a commit that referenced this issue Jul 12, 2024
Part of #4187.

Adds `CompositeFix::None`, which enables fixer functions to decide not to fix some code.

While I was in the area, I took the liberty of adding some doc comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

No branches or pull requests

1 participant