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

Fix only-arrow-functions #1961

Merged
merged 5 commits into from Jan 4, 2017
Merged

Fix only-arrow-functions #1961

merged 5 commits into from Jan 4, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Jan 1, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update
  • Enable CircleCI for your fork (https://circleci.com/add-projects)

What changes did you make?

  • Allow function declarations with allow-named-functions
    • you could argue, that one could simply use allow-declarations, but function declarations are also named
    • maybe this commit could just be replaced with a documentation update, as I was not expecting the current behaviour by reading the docs
  • fix error location for async or exported functions
  • cache lookup of options

Is there anything you'd like reviewers to focus on?

The ci failure will go away when #1960 get's merged and this branch is rebased ... and it's gone


constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);
this.allowDeclarations = options.ruleArguments.indexOf(OPTION_ALLOW_DECLARATIONS) !== -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use this.hasOption here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

options is already in scope, so why not use it.

I don't have a strong opinion on this one

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go with this.hasOption for consistency

}
}

private getFunctionKeyword(node: ts.FunctionLikeDeclaration): ts.Node {
Copy link
Contributor

Choose a reason for hiding this comment

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

One-liner as of #1962

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I changed it to use the new helper function


constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);
this.allowDeclarations = options.ruleArguments.indexOf(OPTION_ALLOW_DECLARATIONS) !== -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go with this.hasOption for consistency

@nchen63 nchen63 merged commit 9bd2bc0 into palantir:master Jan 4, 2017
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.

None yet

4 participants