progre / tslint-config-airbnb

[Deprecated] A TSLint config for Airbnb JavaScript Style
Other
464 stars 58 forks source link

Use of strict-boolean-expressions #29

Open narkowicz opened 6 years ago

narkowicz commented 6 years ago

The configuration of tslint: strict-boolean-expressions currently prevents using short-circuit evaluation in assignment as per Airbnb's 15.7 no-unneeded-ternary:

This type is not allowed in the operand for the '||' operator because it is always truthy.

There's an issue https://github.com/palantir/tslint/issues/3279 concerning the application of strict-boolean-expressions in different contexts that will hopefully allow excluding assignments - which would support loosening the rule to support compliance with no-unneeded-ternary.

Meanwhile there is no ideal setting, but I've currently overridden the rule to the following:

"strict-boolean-expressions": [
  true,
  "allow-null-union",
  "allow-undefined-union",
  "allow-string",
  "allow-mix"
]

It's not ideal... :neutral_face: but helps compliance with 15.7 for string building and assignment (at the cost of compliance with 15.3), while not allowing numbers prevents using array length without a comparison which seems to be the most common violation of 15.3 at the cost of excluding use of numbers in logical assignment.

I appreciate there are a lot of inconsistencies with the eslint implementation due largely to rule availability and differing coding standards - so mainly just flagging the open tslint issue and more generally interested in your thoughts?

progre commented 6 years ago

I think the ruleset shouldn't to limit the style guide. So I think your rule is good and I think current ruleset should be fixed.

narkowicz commented 6 years ago

If the preference is to make the rule set permissive wherever it might limit adherence to the style guide, at the cost of enforcing strictness elsewhere maybe "allow-number" should also be added to the mix of allowed boolean expressions.

Unfortunately using inline constant strings currently doesn't comply:

let a = foo || 'bar'; :no_entry_sign:

...but you can cast to a string to satisfy the linter like so:

let a = foo || 'bar' as string; :nauseated_face: :confused:

Being more permissive the rule change won't break anyone's linting now - but might catch some out later if restored to be more restrictive in the right contexts for cases of 15.3

progre commented 6 years ago

I look like it is possible without as-cast....?

narkowicz commented 6 years ago

Disable the rule completely? I guess that fits with the permissive approach...

Revisit once palantir/tslint#3279 is resolved.

progre commented 6 years ago

I thought it can release without wait for palantir/tslint#3279. I released it as 5.6.0. I'll keep watching the issue.

progre commented 6 years ago

I look like it is possible without as-cast....?

When I tried it today, the error was displayed. It seems this tslint's issue isn't easily fixed. I have no idea when it will be fixed, so I thought it should be disabled.

    "strict-boolean-expressions": false
progre commented 6 years ago

The rule was removed. It is going to be reverted when tslint's issue is fixed.