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

Create a whitelist for id-length rule #40

Open
karol-majewski opened this issue Feb 13, 2018 · 4 comments
Open

Create a whitelist for id-length rule #40

karol-majewski opened this issue Feb 13, 2018 · 4 comments

Comments

@karol-majewski
Copy link
Contributor

Hello and thank you for your work.

The id-length rule is great for forbidding one-letter variable names. However, there are cases where one-letter symbols would actually be preferred: type parameters (GenericTypeAnnotation nodes) and holes.

Holes

Array
  .from({ length: 10 })
  .map((_, index) => index);

Type parameters

function unique<T>(collection: T[]): T[] {
  return [...new Set(collection)];
}

What do you think about passing a config that would allow them?

"id-length": [true, {
  "check-type-parameters": false,
  "allow": ["_"]
}]
@Glavin001
Copy link
Owner

Glavin001 commented Feb 13, 2018

id-length supports exceptions:

https://github.com/Glavin001/tslint-clean-code/blob/master/src/tests/IdLengthRuleTests.ts#L89-L93

[
  true,
  {
    exceptions: ["x", "y", "f", "c"]
  }
];

https://github.com/Glavin001/tslint-clean-code/blob/master/src/tests/IdLengthRuleTests.ts#L135

[true, ["x", "y", "f", "c"]];

Does this resolve your issue? 😃

@karol-majewski
Copy link
Contributor Author

@Glavin001 It mostly does, thank you! I looked here and thought options: null means there is no config available.

@Glavin001
Copy link
Owner

Oh, you're right! I think that's a bug in the metadata. Pull Requests welcome 😃

@Glavin001 Glavin001 added bug and removed enhancement labels Feb 13, 2018
@karol-majewski
Copy link
Contributor Author

karol-majewski commented May 11, 2018

@Glavin001 From what I can see, all valid options (except for the implicit boolean at the beginning) look like this:

optionExamples: [
    [true],
    [true, 2],
    [true, ['x', 'y', 'f', 'c']],
    [true, 2, ['x', 'y', 'f', 'c']],
    [
        true,
        {
            min: 2,
            max: 10,
            exceptions: ['x', 'y', 'f', 'c'],
        },
    ],
],

Given that model and the IRuleMetadata.options description:

The first boolean for whether the rule is enabled or not is already implied. This field describes the options after that boolean. If null, this rule has no options and is not configurable.

I produced the following JSON schema which you can validate on-line:

{
  "$schema": "http://json-schema.org/draft-06/schema#",
  "definitions": {
    "minimum-length": {
      "type": "integer",
      "minimum": 1,
      "default": 2
    },
    "maximum-length": {
      "type": "integer",
      "minimum": 1
    },
    "exceptions": {
      "type": "array",
      "items": {
        "type": "string"
      },
      "minLength": 0,
      "uniqueItems": true
    }
  },
  "type": "array",
  "items": {
    "type": "array",
    "items": {
      "oneOf": [
        {
          "title": "Only the minimum length",
          "$ref": "#/definitions/minimum-length"
        },
        {
          "title": "Only the exceptions array",
          "$ref": "#/definitions/exceptions"
        },
        {
          "title": "Configuration object",
          "type": "object",
          "properties": {
            "min": { "$ref": "#/definitions/minimum-length" },
            "max": { "$ref": "#/definitions/maximum-length" },
            "exceptions": { "$ref": "#/definitions/exceptions" }
          },
          "additionalProperties": false,
          "minProperties": 1
        }
      ]
    },
    "minItems": 1,
    "maxItems": 1
  }
}

The schema is supposed to describe the options after that boolean. My schema works with the below model:

[
  [2],
  [["x", "y", "f", "c"]],
  [
    {
      "min": 2,
      "max": 10,
      "exceptions": ["x", "y", "f", "c"]
    }
  ]
]

but what seems to be problematic is the configuration variant in which one passes both the minumum length and exceptions:

[true, 2, ["x", "y", "f", "c"]]

In that situation, should the the options after that boolean be a tuple of [number, string[]]? I haven't found a TSLint rule that could be configured in a similar fashion. Maybe you have encountered one with the correct docs?

Additionally, the version of TSLint used in the repository (5.1.0) doesn't allow to represent the optionExamples well. It requires all the combinations to consist of strings only.

/**
 * Examples of what a standard config for the rule might look like.
 */
optionExamples?: string[];

The new API allows JSON-like objects:

https://github.com/palantir/tslint/blob/5c86dd4fee6a27643c176530adca02130d3f1634/src/language/rule/rule.ts#L72-L76

It may be necessary to upgrade TSLint in order to deliver rich rule documentation without having to cast it to any type.

karol-majewski added a commit to karol-majewski/tslint-clean-code that referenced this issue Jul 26, 2018
Type assertion is required because the currently used version of TSLint expects a string[]. See Glavin001#40.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants