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

Fix #518: Exclude generic function in TSX file #521

Merged
merged 3 commits into from
Oct 7, 2018
Merged

Fix #518: Exclude generic function in TSX file #521

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0da025c
Ignore .tsx file for no-function-expression rule
br1anchen Oct 6, 2018
30c3b78
Add test case for excluding .tsx file
br1anchen Oct 6, 2018
11fb84e
Add logic to only allow generic type function expression
br1anchen Oct 7, 2018
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
8 changes: 7 additions & 1 deletion src/noFunctionExpressionRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ export class Rule extends Lint.Rules.AbstractRule {
}

class NoFunctionExpressionRuleWalker extends ErrorTolerantWalker {
protected visitSourceFile(node: ts.SourceFile): void {
if (/.*\.tsx/.test(node.fileName) === false) {
Copy link

Choose a reason for hiding this comment

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

This'll accidentally skip non-tsx files in the rare case of someone including .tsx in their file name before the extension, such as the very odd foo.tsx.ts. Let's switch it to just checking if the string ends with the .tsx extension. endsWith is supported in all Node versions we support here.

Copy link

Choose a reason for hiding this comment

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

Also, the issue's conclusion seemed to be that we should still complain on functions in .tsx files and only skip complaining if they have a generic. I think you'll need to still visit those source files, then individually check functions to see if they have a generic type.

Copy link
Contributor Author

@br1anchen br1anchen Oct 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@JoshuaKGoldberg I checked other place which deals with .tsx (AstUtils.ts/getLanguageVariant, etc.), they still use /.*\.tsx/.test(node.fileName), is there any plan to replace them with endsWith as you suggest ?

Copy link

Choose a reason for hiding this comment

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

😲 Good find, that's a bug! Filed #525.

super.visitSourceFile(node);
}
}

protected visitFunctionExpression(node: ts.FunctionExpression): void {
const walker = new SingleFunctionWalker(this.getSourceFile(), this.getOptions());
node.getChildren().forEach((child: ts.Node) => {
Expand Down Expand Up @@ -62,4 +68,4 @@ class SingleFunctionWalker extends ErrorTolerantWalker {
// do not visit inner blocks
}
/* tslint:enable:no-empty */
}
}
6 changes: 5 additions & 1 deletion src/tests/NoFunctionExpressionRuleTests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ describe('noFunctionExpressionRule', (): void => {
TestHelper.assertViolations(ruleName, script, []);
});

it('should pass on function expression within a .tsx file', (): void => {
TestHelper.assertViolations(ruleName, 'test-data/NoFunctionExpressionTests-passing.tsx', [ ]);
});

it('should fail on function expression', (): void => {
const script: string = `
var x = function() {
Expand Down Expand Up @@ -94,4 +98,4 @@ describe('noFunctionExpressionRule', (): void => {
}
]);
});
});
});
3 changes: 3 additions & 0 deletions test-data/NoFunctionExpressionTests-passing.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const myFunction = function() {
return 1 + 1;
};