-
Notifications
You must be signed in to change notification settings - Fork 889
Added allow-empty-catch option to no-empty #2886
Conversation
Allows catch blocks to be empty. Fixes #1088.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with suggestions
src/rules/noEmptyRule.ts
Outdated
@@ -59,6 +71,16 @@ function walk(ctx: Lint.WalkContext<void>) { | |||
}); | |||
} | |||
|
|||
function isExcluded(node: ts.Node, options: Options): boolean { | |||
const { parent } = node; | |||
if (parent === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parent
will never be undefined here
src/rules/noEmptyRule.ts
Outdated
} | ||
|
||
return isExcludedConstructor(parent) | ||
|| (options.allowEmptyCatch === true && parent.kind === ts.SyntaxKind.CatchClause); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can simply rename isExcludedConstructor
and add this condition to its return statement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ehh, I actually prefer the explicitness. But sure.
src/rules/noEmptyRule.ts
Outdated
const ALLOW_EMPTY_CATCH = "allow-empty-catch"; | ||
|
||
interface Options { | ||
allowEmptyCatch?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't need to be optional
PR checklist
Overview of change:
Allows catch blocks to be empty.
Fixes #1088.
CHANGELOG.md entry:
[enhancement] Added allow-empty-catch option to
no-empty