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

Add no-internal-module rule #517

Merged
merged 4 commits into from
Jul 24, 2015
Merged

Conversation

vvakame
Copy link
Contributor

@vvakame vvakame commented Jul 22, 2015

implement #513

Review on Reviewable


class NoInternalModuleWalker extends Lint.RuleWalker {
public visitModuleDeclaration(node: ts.ModuleDeclaration) {
let name = node.name.getText();
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you should look at the raw text here. The type of node.name is Identifier | LiteralExpression. So you should be able to simplify this to:

        if (Lint.isNodeFlagSet(node, ts.NodeFlags.Namespace)) {
            // ok namespace
        } else if (node.name.syntaxKind === ts.SyntaxKind.Identifier) {
            // for external modules, node.name will be a LiteralExpression instead of Identifier
            this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool! I added a commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

This name variable is no longer used. Linting should've picked that up, maybe it's a bug... Regardless, let's remove the let name...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, sorry!

@vvakame vvakame force-pushed the no-internal-module branch from efc3458 to 39c3e92 Compare July 24, 2015 12:33
@adidahiya
Copy link
Contributor

sweet, thanks!

adidahiya added a commit that referenced this pull request Jul 24, 2015
@adidahiya adidahiya merged commit 1fc12c1 into palantir:master Jul 24, 2015
@vvakame vvakame deleted the no-internal-module branch July 25, 2015 01:10
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.

2 participants