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

Feature request: how to fail a build for particular code fixes? #25105

Closed
4 tasks done
JoshuaKGoldberg opened this issue Jun 20, 2018 · 2 comments
Closed
4 tasks done
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript

Comments

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jun 20, 2018

Search Terms

code fix warn error fail build lint tslint

Suggestion

Some code fixes, such as @elizabethdinella's super exciting #25082, are important enough that some projects would want to fail a build if any code actions of that particular type are suggested. Fixes such as async/await conversion and unused variable removals would be helpful, while optional fixes such as converting members to getter+setter pairs likely wouldn't be. Projects would likely need to configure their own preferences.

Use Cases

Suppose you have a repository that's been fully converted to async/await and someone sends a pull request containing a .then can can be converted away using a code fix. We would want to fail the build for the user not having done so. How?

Examples

One option would be for TypeScript to have a setting on the CLI & tsconfig.json for code fixes to emit as diagnostic errors. This would require each code fix to have a friendly name:

{
  "compilerOptions": {
    "codeFixStrictness": {
      "async-await-from-then": "error"
    },
  },
  "include": ["src/**/*.ts"]
}

Alternately, we could write TSLint rules for each code fix (or one rule with settings & AST walker logic?) per code fix. no-unused-variable uses the diagnostics emit to read TS-native warnings; we could do a similar thing with crawling the tree.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript / JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. new expression-level syntax)
@RyanCavanaugh
Copy link
Member

I definitely get the appeal of this but have some real technical concerns. The codefixes are really only acceptable performance-wise because we only run them on a small piece of code at once; running a large set of fix rules over a large codebase would probably be quite slow. There's also the pedestrian problem that the codefix code isn't in tsc.js, but obviously we can change that. We also freely rev the codefix code to include new fix scenarios and it'd be troublesome to have that qualify as a "breaking change".

It'd really make more sense to do this as an out-of-band tool since you'd presumably also want to actually run the fix and then maybe do some formatting afterwards. That's a lot easier to do from a commandline tool that's set up to do it with your team's logic. If someone took this on to make a high-quality high-config tool we could have it be a TS-specific kind of "codemod". We'd probably even use it ourselves to fix stuff on DefinitelyTyped.

I'd like to hear more about what people might use this for, though.

@mhegazy mhegazy added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Jun 20, 2018
@RyanCavanaugh RyanCavanaugh added Out of Scope This idea sits outside of the TypeScript language design constraints and removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jun 25, 2021
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 25, 2021

This hasn't gathered much feedback and, empirically, no one has yet bothered to write the tool that would make this happen, so this doesn't look like a good fit. @iisaduan is working on a tool that will do something similar, though, so cc'ing her for FYI on a possible scenario

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants