Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

export-name: detect export declarations #443

Merged
merged 2 commits into from
Jul 5, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions src/exportNameRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,26 +46,43 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

function isExportedDeclaration(element: ts.Statement): boolean {

Choose a reason for hiding this comment

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

Just a suggestion: you may wish to file issues on https://github.com/ajafff/tsutils to add these there, as they seem useful. Not a blocking issue for this pull request at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You got it: ajafff/tsutils#68 - feel free to comment there if it's not clear or not quite what you expected.

return AstUtils.hasModifier(element.modifiers, ts.SyntaxKind.ExportKeyword);
}

type ExportStatement = ts.ExportDeclaration | ts.ExportAssignment;
function isExportStatement(node: ts.Statement): node is ExportStatement {
return ts.isExportAssignment(node) || ts.isExportDeclaration(node);
}

function getExportsFromStatement(node: ExportStatement): [string, ts.Node][] {
if (ts.isExportAssignment(node)) {
return [[node.expression.getText(), node.expression]];
} else {
const symbolAndNodes: [string, ts.Node][] = [];
node.exportClause.elements.forEach(e => {
symbolAndNodes.push([e.name.getText(), node]);
});
return symbolAndNodes;
}
}

export class ExportNameWalker extends ErrorTolerantWalker {
protected visitSourceFile(node: ts.SourceFile): void {

// look for single export assignment from file first
const singleExport: ts.Statement[] = node.statements.filter((element: ts.Statement): boolean => {
return element.kind === ts.SyntaxKind.ExportAssignment;
});
const singleExport = node.statements.filter(isExportStatement);
if (singleExport.length === 1) {
const exportAssignment: ts.ExportAssignment = <ts.ExportAssignment>singleExport[0];
this.validateExport(exportAssignment.expression.getText(), exportAssignment.expression);
const symbolsAndNodes = getExportsFromStatement(singleExport[0]);
if (symbolsAndNodes.length === 1) {
this.validateExport(symbolsAndNodes[0][0], symbolsAndNodes[0][1]);
}

return; // there is a single export and it is valid, so do not proceed
}

let exportedTopLevelElements: ts.Statement[] = [];

// exports are normally declared at the top level
node.statements.forEach((element: ts.Statement): void => {
const exportStatements: ts.Statement[] = this.getExportStatements(element);
exportedTopLevelElements = exportedTopLevelElements.concat(exportStatements);
});
let exportedTopLevelElements: ts.Statement[] = node.statements.filter(isExportedDeclaration);

// exports might be hidden inside a namespace
if (exportedTopLevelElements.length === 0) {
Expand All @@ -84,27 +101,12 @@ export class ExportNameWalker extends ErrorTolerantWalker {
// modules may be nested so recur into the structure
return this.getExportStatementsWithinModules(<ts.ModuleDeclaration>moduleDeclaration.body);
} else if (moduleDeclaration.body.kind === ts.SyntaxKind.ModuleBlock) {
let exportStatements: ts.Statement[] = [];
const moduleBlock: ts.ModuleBlock = <ts.ModuleBlock>moduleDeclaration.body;
moduleBlock.statements.forEach((element: ts.Statement): void => {
exportStatements = exportStatements.concat(this.getExportStatements(element));
});
return exportStatements;
return moduleBlock.statements.filter(isExportedDeclaration);
}
return null;
}

private getExportStatements(element: ts.Statement): ts.Statement[] {
const exportStatements: ts.Statement[] = [];
if (element.kind === ts.SyntaxKind.ExportAssignment) {
const exportAssignment: ts.ExportAssignment = <ts.ExportAssignment>element;
this.validateExport(exportAssignment.expression.getText(), exportAssignment.expression);
} else if (AstUtils.hasModifier(element.modifiers, ts.SyntaxKind.ExportKeyword)) {
exportStatements.push(element);
}
return exportStatements;
}

private validateExportedElements(exportedElements: ts.Statement[]): void {
// only validate the exported elements when a single export statement is made
if (exportedElements.length === 1) {
Expand Down
20 changes: 19 additions & 1 deletion src/tests/ExportNameRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,24 @@ describe('exportNameRule', () : void => {
]);
});

it('when mis-named function is exported in a separate statement', () : void => {

Choose a reason for hiding this comment

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

Good test for the case of complaining. There should also be a corresponding test for the case of the user getting it right, e.g. file.ts having export { file };.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in new commit, good catch!

// TestHelper assumes that all scripts are within file.ts
const script : string = `
function Example3a() {}
export { Example3a };
`;

TestHelper.assertViolations(ruleName, script, [
{
"failure": "The exported module or identifier name must match the file name. Found: file.ts and Example3a",
"name": "file.ts",
"ruleName": "export-name",
"startPosition": { "character": 17, "line": 3 }
}

]);
});

it('when mis-named let defined variable is exported', () : void => {
// TestHelper assumes that all scripts are within file.ts
const script : string = `
Expand Down Expand Up @@ -364,4 +382,4 @@ describe('exportNameRule', () : void => {
});

});
});
});