Conversation
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.
Looks mostly good. Minor issues
@@ -0,0 +1,296 @@ | |||
/** | |||
* @license | |||
* Copyright 2013 Palantir Technologies, Inc. |
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.
Use current year
super.visitModuleDeclaration(node); | ||
} | ||
|
||
public visitInterfaceDeclaration(node: ts.InterfaceDeclaration): void { |
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.
don't need void
|
||
import * as Lint from "../index"; | ||
|
||
import { getOverloadKey } from "./adjacentOverloadSignaturesRule"; |
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.
having a dependency on a different rule is iffy, but I can see why you did it. I think this is ok
this.checkOverloads(members, getOverloadName, typeParameters); | ||
function getOverloadName(member: ts.TypeElement | ts.ClassElement) { | ||
const key = getOverloadKey(member); | ||
return key === undefined ? undefined : { signature: member as any as ts.SignatureDeclaration, key }; |
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.
If a non-undefined value means member
is a SignatureDeclaration
, it would be more clear if a function isSignatureDeclaration
was factored out of getOverloadKey
@andy-ms thanks! |
PR checklist
What changes did you make?
Added the
unified-signatures
rule. This encourages users to combine overloads where possible.Is there anything you'd like reviewers to focus on?
There may be some test cases I missed. This was originally for declaration files only, but should work outside of them.
Re: #1844 (comment) ("How would this work for more complex signatures"), I added a test labelled
// No error if they differ by 2 or more parameters.
.This is a rewrite of the one on types-publisher and not just a direct copy, because it needed to handle more cases, and could re-use code from
adjacent-overload-signatures
.