Skip to content

@noSelf changed to not affect merged namespaces #689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Perryvw merged 1 commit into from
Jul 26, 2019
Merged

@noSelf changed to not affect merged namespaces #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 18 additions & 14 deletions src/TSHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,19 +337,25 @@ export function getCustomDecorators(type: ts.Type, checker: ts.TypeChecker): Map
return decMap;
}

export function getCustomNodeDirectives(node: ts.Node): Map<DecoratorKind, Decorator> {
const directivesMap = new Map<DecoratorKind, Decorator>();

ts.getJSDocTags(node).forEach(tag => {
const tagName = tag.tagName.escapedText as string;
if (Decorator.isValid(tagName)) {
const dec = new Decorator(tagName, tag.comment ? tag.comment.split(" ") : []);
directivesMap.set(dec.kind, dec);
}
});

return directivesMap;
}

export function getCustomFileDirectives(file: ts.SourceFile): Map<DecoratorKind, Decorator> {
const decMap = new Map<DecoratorKind, Decorator>();
if (file.statements.length > 0) {
const tags = ts.getJSDocTags(file.statements[0]);
for (const tag of tags) {
const tagName = tag.tagName.escapedText as string;
if (Decorator.isValid(tagName)) {
const dec = new Decorator(tagName, tag.comment ? tag.comment.split(" ") : []);
decMap.set(dec.kind, dec);
}
}
return getCustomNodeDirectives(file.statements[0]);
}
return decMap;
return new Map();
}

export function getCustomSignatureDirectives(
Expand Down Expand Up @@ -596,8 +602,7 @@ export function hasNoSelfAncestor(declaration: ts.Declaration, checker: ts.TypeC
if (ts.isSourceFile(scopeDeclaration)) {
return getCustomFileDirectives(scopeDeclaration).has(DecoratorKind.NoSelfInFile);
}
const scopeType = checker.getTypeAtLocation(scopeDeclaration);
if (scopeType && getCustomDecorators(scopeType, checker).has(DecoratorKind.NoSelf)) {
if (getCustomNodeDirectives(scopeDeclaration).has(DecoratorKind.NoSelf)) {
return true;
}
return hasNoSelfAncestor(scopeDeclaration, checker);
Expand Down Expand Up @@ -634,8 +639,7 @@ export function getDeclarationContextType(
return ContextType.NonVoid;
}

const scopeType = checker.getTypeAtLocation(scopeDeclaration);
if (scopeType && getCustomDecorators(scopeType, checker).has(DecoratorKind.NoSelf)) {
if (getCustomNodeDirectives(scopeDeclaration).has(DecoratorKind.NoSelf)) {
return ContextType.Void;
}
return ContextType.NonVoid;
Expand Down
32 changes: 32 additions & 0 deletions test/unit/assignments/functionPermutations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@ export const selfTestFunctions: TestFunction[] = [
}
const anonFunctionNestedInNoSelfClass = (new AnonFunctionNestedInNoSelfClass).method();`,
},
{
value: "anonMethodClassMergedNoSelfNS.method",
definition: `class AnonMethodClassMergedNoSelfNS { method(s: string): string { return s; } }
/** @noSelf */ namespace AnonMethodClassMergedNoSelfNS { export function nsFunc(s: string) { return s; } }
Copy link
Copy Markdown
Member

@Perryvw Perryvw Jul 25, 2019

Choose a reason for hiding this comment

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

Do these actually fail without the transformer change?

Copy link
Copy Markdown
Collaborator Author

@tomblind tomblind Jul 25, 2019

Choose a reason for hiding this comment

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

Yes, because the class method will have it's self stripped due to the namespace using @noSelf

Copy link
Copy Markdown
Member

@Perryvw Perryvw Jul 25, 2019

Choose a reason for hiding this comment

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

But the call to the function will adjust accordingly?

Copy link
Copy Markdown
Collaborator Author

@tomblind tomblind Jul 25, 2019

Choose a reason for hiding this comment

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

Yes, but the assignment test would fail since it would try to assign the method to a type with a self parameter. Also, those tests actually call the function and verify parameters are in correct order. That could probably be moved out to separate tests, but that seems out of scope for this PR.

const anonMethodClassMergedNoSelfNS = new AnonMethodClassMergedNoSelfNS();`,
},
{
value: "AnonFuncNSMergedNoSelfClass.nsFunc",
definition: `/** @noSelf */ class AnonFuncNSMergedNoSelfClass { method(s: string): string { return s; } }
namespace AnonFuncNSMergedNoSelfClass { export function nsFunc(s: string) { return s; } }`,
},
{
value: "SelfAnonFuncNSMergedNoSelfNS.nsFuncSelf",
definition: `namespace SelfAnonFuncNSMergedNoSelfNS { export function nsFuncSelf(s: string): string { return s; } }
/** @noSelf */ namespace SelfAnonFuncNSMergedNoSelfNS { export function nsFuncNoSelf(s: string) { return s; } }`,
},
];

export const noSelfTestFunctions: TestFunction[] = [
Expand Down Expand Up @@ -263,6 +279,22 @@ export const noSelfTestFunctions: TestFunction[] = [
const anonFunctionNestedInClassInNoSelfNs =
(new AnonFunctionNestedInClassInNoSelfNs.AnonFunctionNestedInClass).method();`,
},
{
value: "noSelfAnonMethodClassMergedNS.method",
definition: `/** @noSelf */ class NoSelfAnonMethodClassMergedNS { method(s: string): string { return s; } }
namespace NoSelfAnonMethodClassMergedNS { export function nsFunc(s: string) { return s; } }
const noSelfAnonMethodClassMergedNS = new NoSelfAnonMethodClassMergedNS();`,
},
{
value: "NoSelfAnonFuncNSMergedClass.nsFunc",
definition: `class NoSelfAnonFuncNSMergedClass { method(s: string): string { return s; } }
/** @noSelf */ namespace NoSelfAnonFuncNSMergedClass { export function nsFunc(s: string) { return s; } }`,
},
{
value: "NoSelfAnonFuncNSMergedSelfNS.nsFuncNoSelf",
definition: `namespace NoSelfAnonFuncNSMergedSelfNS { export function nsFuncSelf(s: string): string { return s; } }
/** @noSelf */ namespace NoSelfAnonFuncNSMergedSelfNS { export function nsFuncNoSelf(s: string) { return s; } }`,
},
];

const noSelfInFileTestFunctions: TestFunction[] = [
Expand Down