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

Repo: Rule [options] parameter should be non-nullable if defaultOptions exists #5439

Open
JoshuaKGoldberg opened this issue Aug 7, 2022 · 3 comments
Labels
blocked by external API Blocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API enhancement New feature or request package: utils Issues related to the @typescript-eslint/utils package repo maintenance things to do with maintenance of the repo, and not with code/docs

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Aug 7, 2022

Suggestion

Right now, if a rule is created through util.createRule with defaultOptions, the create function's second parameter is an array whose first element is still Options | undefined. That means folks have to !. For example, in #5327:

create(context, [_options]) {
const options = _options!;

Unless I've grossly misinterpreted the rule options, those ! should be unnecessary, right?

Edit: here's a TypeScript playground with an isolated repro.

@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for maintainers to take a look repo maintenance things to do with maintenance of the repo, and not with code/docs labels Aug 7, 2022
@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Aug 13, 2022
@JoshuaKGoldberg JoshuaKGoldberg added this to the 8.0.0 milestone Feb 3, 2024
@JoshuaKGoldberg
Copy link
Member Author

We've deleted lines-around-comment in the v8 branch. A new still-in-the-repo example is consistent-type-imports:

defaultOptions: [
{
prefer: 'type-imports',
disallowTypeAnnotations: true,
fixStyle: 'separate-type-imports',
},
],
create(context, [option]) {
const prefer = option.prefer ?? 'type-imports';

@JoshuaKGoldberg
Copy link
Member Author

JoshuaKGoldberg commented Apr 29, 2024

#9025 touches a lot of the same interfaces, so marking as blocked on that one till it's merged.

Edit: ready to go 🙂

@JoshuaKGoldberg JoshuaKGoldberg added the blocked by another PR PRs which are ready to go but waiting on another PR label Apr 29, 2024
@JoshuaKGoldberg JoshuaKGoldberg added enhancement New feature or request package: utils Issues related to the @typescript-eslint/utils package and removed blocked by another PR PRs which are ready to go but waiting on another PR labels May 28, 2024
@JoshuaKGoldberg
Copy link
Member Author

I took a run at this today. It was, as expected, very tricky to try to get to work. Removing from the v8 milestone as there are two upstream ecosystem work items that could land first:

On that latter list item, RuleCreator's createNamedRule is pretty complex right now:

// This function will get much easier to call when this is merged https://github.com/Microsoft/TypeScript/pull/26349
// TODO - when the above PR lands; add type checking for the context.report `data` property
return function createNamedRule<
Options extends readonly unknown[],
MessageIds extends string,
>({

Having to add a potential third type parameter for OptionDefaults is ... even more complexity. Especially since we currently sometimes need to be explicit about types in createRule calls:

export default createRule<Options, MessageId>({

Marking as blocked on external items. 😞

@JoshuaKGoldberg JoshuaKGoldberg added blocked by external API Blocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API and removed accepting prs Go ahead, send a pull request that resolves this issue labels Jun 6, 2024
@JoshuaKGoldberg JoshuaKGoldberg removed this from the 8.0.0 milestone Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked by external API Blocked by a tool we depend on exposing an API, such as TypeScript's Type Relationship API enhancement New feature or request package: utils Issues related to the @typescript-eslint/utils package repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Development

No branches or pull requests

2 participants