xojs / eslint-config-xo-typescript

ESLint shareable config for TypeScript to be used with eslint-config-xo
MIT License
170 stars 25 forks source link

Variable Formatting: global constants #42

Open EdJoPaTo opened 3 years ago

EdJoPaTo commented 3 years ago

Related to the rule @typescript-eslint/naming-convention.

I regularly use global constants with UPPER_CASE style for constants that wont change in runtime. Not sure how to exactly specify that in the rules but I first wanted to suggest something like that here before checking on how that could be done.

Something like these:

const DIRECTORY = './events'
const DEFAULT_OPTIONS: Options = {
  …
}

If thats something that is not liked here: How would you propose to write constants like this instead?

sindresorhus commented 3 years ago

It's intentional: https://github.com/eslint/eslint/issues/10605

sindresorhus commented 3 years ago

@fregante What are your thoughts about this?

fregante commented 3 years ago

Perhaps they should be allowed only at the top level. They're pretty common, even though I don't particularly see the need for that.

EdJoPaTo commented 3 years ago

I'm not sure if they are useful in libraries as they objective is to be configurable. But I have a bunch of them in projects that running somewhere doing something.

As a "need" I searched for some examples from my code (rg "^(?:export )?const [A-Z][A-Z_]+" --glob "*.ts"):

in general: stuff that wont change at runtime / compile time constants

I can use camelcase here but I am kinda used to seeing constants directly. Also they are conventions on other c derived languages (example). I am relatively sure I am not the only xo user using them. Also my other currently used language (Rust) produces a warning in that case: non-upper-case-globals but in contrast to TypeScript it knows when something is really const or not.

sindresorhus commented 2 years ago

Yeah, I guess upper-case is a good way to make it clear that the value is completely static. PR welcome.

sindresorhus commented 2 years ago

Perhaps they should be allowed only at the top level. They're pretty common, even though I don't particularly see the need for that.

Ideally, yes, but there's no way to enforce that with this rule.

EdJoPaTo commented 2 years ago

There is. The first part of the rule description even states that:

such as by enforcing all private properties begin with an _, and all global-level constants are written in UPPER_CASE.

Selectors → modifiers → global

sindresorhus commented 2 years ago

Oh great. I missed that.

EdJoPaTo commented 2 years ago

I tested around with the following but it didnt seem to work… Not sure why…

{
    selector: 'variable',
    format: [
        'strictCamelCase',
        'UPPER_CASE'
    ],
    modifiers: [
        'global',
        'const'
    ]
},
kestrelnova commented 2 years ago

It's because of how the rule prioritizes the selectors. If you have two selectors for variable, one with the filter option and one without, a variable name will always be checked against the one with the filter first.

So because of that, the very first selector in the XO rule ends up overriding any other variable selectors added later (unless they also have filters). Because it has a filter, a variable name will always be checked against it first, and the name will always match because the only criteria is that it doesn't have a hyphen or space (which variables can't have anyway).

There are a few ways to get around this, but the simplest way is just to remove the filter entirely. Properties with hyphens or spaces will still be exempt from camelCase because of the later requiresQuotes selector; I'm not sure if the filter was intended to do anything else?

Once that's removed, the above code for global constants will mostly work as intended. The only issue is that it does conflict with another selector for booleans. Since that one specifies types, it has a higher priority than selectors that only specify modifiers, so any booleans will get matched there first, global constants or not.

(I don't think that boolean rule actually triggers at all right now, because of the same issue with the filter. I didn't get any errors from it until I removed the filter, and then I suddenly got a whole bunch.)

So if you want global constant booleans to have the required prefixes and potentially be UPPER_CASE, you need to add a special case for that, something like this:

{
  selector: 'variable',
  types: ['boolean'],
  modifiers: ['const', 'global'],
  format: ['UPPER_CASE'],
  prefix: ['IS_', 'HAS_', 'CAN_', 'SHOULD_', 'WILL_', 'DID_'],
  filter: '[A-Z]{2,}'
},

Then adding everything together should get the right output.

maxpain commented 2 years ago

Any updates on this?

ollebergkvist commented 1 month ago

Did someone find a way to do this?