Skip to content

[naming-convention] modifier global - Allow selecting only global variables #2318

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.

Sign up for GitHub

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

Jump to bottom
Closed
glen-84 opened this issue Jul 22, 2020 · 6 comments · Fixed by #2808
Closed

[naming-convention] modifier global - Allow selecting only global variables #2318

glen-84 opened this issue Jul 22, 2020 · 6 comments · Fixed by #2808
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@glen-84
Copy link
Contributor

glen-84 commented Jul 22, 2020

Similar to #2239.

I'd like to be able to do something similar to this, i.e.:

// require all global constants to be camelCase or UPPER_CASE
// all other variables and functions still need to be camelCase
{"selector": "variable", "modifiers": ["global", "const"], "format": ["camelCase", "UPPER_CASE"]}
@glen-84 glen-84 added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jul 22, 2020
@bradzacher bradzacher added enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed triage Waiting for team members to take a look labels Jul 22, 2020
@InsOpDe
Copy link

InsOpDe commented Aug 5, 2020

Its funny because the description of the naming-convention rule states

Additionally, a well-designed style guide can help communicate intent, such as by enforcing all private properties begin with an _, and all global-level constants are written in UPPER_CASE.

and yet its not able to enforce this rule

@bradzacher
Copy link
Member

Happy to accept a PR if you want it.

@phaux
Copy link
Contributor

phaux commented Aug 7, 2020

Maybe the name "topLevel" or something similar would be more appropriate for the modifier? When I hear "global" I think something attached to the window object. The convention I personally use is to name every top level or exported const with upper snake case, but use camel case for the ones in blocks, functions, classes, etc.

@phaux
Copy link
Contributor

phaux commented Aug 7, 2020

Unless you actually meant "global" as in attached to the window, but then it can't be a "const" (at least in modules)

@glen-84 glen-84 mentioned this issue Sep 28, 2020
Closed
3 tasks
@bradzacher bradzacher added the good first issue Good for newcomers label Sep 28, 2020
@bradzacher
Copy link
Member

This should be relatively easy to do, if anyone wants to try.

  • define a new modifier
  • when you encounter a variable definition - get the scope (context.getScope())
  • check scope.type is either global or module

@infacto
Copy link

infacto commented Sep 29, 2020 •
edited
Loading

Just to visualize a use case:

random-file.ts

// Global variable allowed to be UPPER_CASE.
const MY_VALUE = 42;

export class MyClass {
  public myMethod(): void {
    // This local const (variable) is NOT allowed.
    const ANOTHER_VALUE = 27;

    // This is allowed.
    const anotherValue = 13;
  }
}

Yes this is not really "global" But for the file scope. It depends on the build system and use of the files. So, we could name it "global".
I don't know if we should cover real global variables in e.g. the window object. Because it's unusual in TypeScript.

Since const has different meaning in TypeScript then in other languages. If a variable (let) is never reassigned const should be used. (rule prefer-const) I only want strictCamelCase for that variables. But allow UPPER_CASE for "real" constants and e.g. static readonly (which already work) The current workaround is either disallow it or allow both cases and mention it in the code reviews.

@jharwig jharwig mentioned this issue Oct 20, 2020
Closed
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
cbac387 
…lobal`, and `destructured` modifiers - add selectors `classProperty`, `objectLiteralProperty`, `typeProperty`, `classMethod`, `objectLiteralMethod`, `typeMethod`

Fixes #2239
Fixes #2512
Fixes #2318
Fixes #1477

Big update to add a bunch of stuff.
I couldn't be bothered splitting this out into separate PRs.
@bradzacher bradzacher mentioned this issue Nov 24, 2020
Closed
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
5323fff 
…lobal`, and `destructured` modifiers - add selectors `classProperty`, `objectLiteralProperty`, `typeProperty`, `classMethod`, `objectLiteralMethod`, `typeMethod`

Fixes #2239
Fixes #2512
Fixes #2318
Fixes #1477

Big update to add a bunch of stuff.
I couldn't be bothered splitting this out into separate PRs.

Adds the following modifiers:
- `exported` - matches anything that is exported from the module.
- `global` - matches a variable/function declared in the top-level scope.
- `destructured` - matches a variable declared via an object destructuring pattern (`const {x, ignored: y, z = 2}`).

Adds the following selectors (self explanatory - just breaking the selectors up):
- `classProperty`
- `objectLiteralProperty`
- `typeProperty`
- `classMethod`
- `objectLiteralMethod`
- `typeMethod`

Converts
- `property` to a meta selector for `classProperty`, `objectLiteralProperty`, `typeProperty`
- `method` to a meta selector for `classMethod`, `objectLiteralMethod`, `typeMethod`
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
3c91b53 
…lobal`, and `destructured` modifiers - add selectors `classProperty`, `objectLiteralProperty`, `typeProperty`, `classMethod`, `objectLiteralMethod`, `typeMethod`

Fixes #2239
Fixes #2512
Fixes #2318
Fixes #1477

Big update to add a bunch of stuff.
I couldn't be bothered splitting this out into separate PRs.

Adds the following modifiers:
- `exported` - matches anything that is exported from the module.
- `global` - matches a variable/function declared in the top-level scope.
- `destructured` - matches a variable declared via an object destructuring pattern (`const {x, ignored: y, z = 2}`).

Adds the following selectors (self explanatory - just breaking the selectors up):
- `classProperty`
- `objectLiteralProperty`
- `typeProperty`
- `classMethod`
- `objectLiteralMethod`
- `typeMethod`

Converts
- `property` to a meta selector for `classProperty`, `objectLiteralProperty`, `typeProperty`
- `method` to a meta selector for `classMethod`, `objectLiteralMethod`, `typeMethod`
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
3adb1dc 
…lobal`, and `destructured`

Fixes #2239
Fixes #2512
Fixes #2318
Closes #2802

Adds the following modifiers:
- `exported` - matches anything that is exported from the module.
- `global` - matches a variable/function declared in the top-level scope.
- `destructured` - matches a variable declared via an object destructuring pattern (`const {x, ignored: y, z = 2}`).
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
d6a08d4 
…lobal`, and `destructured`

Fixes #2239
Fixes #2512
Fixes #2318
Closes #2802

Adds the following modifiers:
- `exported` - matches anything that is exported from the module.
- `global` - matches a variable/function declared in the top-level scope.
- `destructured` - matches a variable declared via an object destructuring pattern (`const {x, z = 2}`).
@bradzacher bradzacher mentioned this issue Nov 24, 2020
Merged
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
c4cb9f2 
…lobal`, and `destructured`

Fixes #2239
Fixes #2512
Fixes #2318
Closes #2802

Adds the following modifiers:
- `exported` - matches anything that is exported from the module.
- `global` - matches a variable/function declared in the top-level scope.
- `destructured` - matches a variable declared via an object destructuring pattern (`const {x, z = 2}`).
bradzacher added a commit that referenced this issue Nov 24, 2020
@bradzacher
feat(eslint-plugin): [naming-convention] add modifiers exported, `g…
fb254a1 
…lobal`, and `destructured` (#2808)

Fixes #2239
Fixes #2512
Fixes #2318
Closes #2802

Adds the following modifiers:
- `exported` - matches anything that is exported from the module.
- `global` - matches a variable/function declared in the top-level scope.
- `destructured` - matches a variable declared via an object destructuring pattern (`const {x, z = 2}`).
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
5 participants
@glen-84 @phaux @bradzacher @InsOpDe @infacto