tslint-clean-code icon indicating copy to clipboard operation
tslint-clean-code copied to clipboard

Create a whitelist for id-length rule

Open karol-majewski opened this issue 8 years ago • 4 comments

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": ["_"]
}]

karol-majewski avatar Feb 13 '18 10:02 karol-majewski

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? 😃

Glavin001 avatar Feb 13 '18 14:02 Glavin001

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

karol-majewski avatar Feb 13 '18 16:02 karol-majewski

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

Glavin001 avatar Feb 13 '18 16:02 Glavin001

@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 avatar May 11 '18 21:05 karol-majewski