Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

variable-names: allow-aliases. #1510

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

mprobst
Copy link
Contributor

@mprobst mprobst commented Aug 23, 2016

This option allows aliasing symbols, as long as the left hand side name
is identical to the right hand side name.

The use case is interoperating with legacy code that uses namespaces,
but still being able to abbreviate long namespaces in an ES6 module:

const SomeClass = some.long.namespace.SomeClass;

Another use case is exposing enums or other PascalCased objects to a
template in an Angular application (or similar environments):

class MyCtrl {
  SomeEnum = SomeEnum;
}

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @mprobst! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@mprobst
Copy link
Contributor Author

mprobst commented Aug 23, 2016

Company CLA has been mailed out, as far as I know.

@mprobst
Copy link
Contributor Author

mprobst commented Aug 23, 2016

Not sure what's going on with the CI failure, that seems unrelated to my change?

@jkillian
Copy link
Contributor

Sorry about that, #1511 should've fixed things now if you merge in master

@mprobst
Copy link
Contributor Author

mprobst commented Aug 24, 2016

Rebased.

Re CLA, Google has signed the Corporate CLA for all Google employees (including me), and mailed it to opensource@palantir.com.

@adidahiya
Copy link
Contributor

Confirmed receipt of the Corporate CLA 👍

@mprobst
Copy link
Contributor Author

mprobst commented Aug 31, 2016

Friendly ping! :-)

@jkillian
Copy link
Contributor

jkillian commented Sep 1, 2016

Sorry for the delay @mprobst. I'm going back on forth if this should just be baked in to the rule instead of another option. I'm hesitant to add too many options, it's sort of a weird set of options as it is. What do you think?

@@ -39,6 +40,7 @@ export class Rule extends Lint.Rules.AbstractRule {
* \`"${OPTION_LEADING_UNDERSCORE}"\` allows underscores at the beginning (only has an effect if "check-format" specified)
* \`"${OPTION_TRAILING_UNDERSCORE}"\` allows underscores at the end. (only has an effect if "check-format" specified)
* \`"${OPTION_ALLOW_PASCAL_CASE}"\` allows PascalCase in addtion to camelCase.
* \`"${OPTION_ALLOW_ALIASES}"\`: does not check the names of aliases, e.g. allows \`let FooBar = FooBar;\`
* \`"${OPTION_BAN_KEYWORDS}"\`: disallows the use of certain TypeScript keywords (\`any\`, \`Number\`, \`number\`, \`String\`,
\`string\`, \`Boolean\`, \`boolean\`, \`undefined\`) as variable or parameter names.`,
options: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep the option, can you add it to the schema below?

This change allows aliasing symbols, as long as the left hand side name
is identical to the right hand side name.

The use case is interoperating with legacy code that uses namespaces,
but still being able to abbreviate long namespaces in an ES6 module:

    const SomeClass = some.long.namespace.SomeClass;

Another use case is exposing enums or other PascalCased objects to a
template in an Angular application (or similar environments):

    class MyCtrl {
      SomeEnum = SomeEnum;
    }
@mprobst mprobst force-pushed the alias-variable-anems branch from 84e64ea to 49cac3f Compare September 2, 2016 01:37
@mprobst
Copy link
Contributor Author

mprobst commented Sep 2, 2016

@jkillian I think you're right. Please take another look, I removed the option and just made this the default behaviour. There's one tricky thing: if you have let {X} = {X: 1};, tslint now no longer warns, but I guess that's WAI? It's a corner case in any case.

@jkillian
Copy link
Contributor

jkillian commented Sep 6, 2016

@mprobst agreed that let {X} = {X: 1} doesn't need to be handled specially. Thanks for the PR!

@jkillian jkillian merged commit 3443e1b into palantir:master Sep 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants