Skip to content

Commit

Permalink
Add jsdoc support for @public/@private/@Protected (#35731)
Browse files Browse the repository at this point in the history 
* Add @private/@protected/@public test

* Fix @declaration

* draft abstraction + one usage

* Fill in necessary parsing etc

* make general getEffectiveModifierFlags

move to utilities, make the right things call it

* reorder public/private/protected

* JS declaration emit works with @public/@private/@Protected

* revert unneeded/incorrect changes

* Update baselines and skip @public/etc when parsing

1. Update the API baselines with the new functions.
2. Do not check for @public/etc during parsing, because parent pointers
aren't set, so non-local tags will be missed; this wrong answer will
then be cached.

* Parser: don't call hasModifier(s) anymore.

Then move jsdoc modifier tag checks into getModifierFlagsNoCache where
they should be. The jsdoc checks are skipped when the parent is
undefined. There are 3 cases when this can happen:

1. The code is in the parser (or a few places in the binder, notably
declareSymbol of module.exports assignments).
2. The node is a source file.
3. The node is synthetic, which I assume to be from the transforms.

It is fine to call getModifierFlags in cases (2) and (3). It is not fine
for (1), so I removed these calls and replaced them with simple
iteration over the modifiers. Worth noting: Ron's uniform node construction
PR removes these calls anyway; this PR just does it early.

* Fix constructor emit

1. Emit protected for protected, which I missed earlier.
2. Emit a constructor, not a property named "constructor".
3. Split declaration emit tests out so that errors are properly reported
there.
  • Loading branch information
@sandersn
sandersn committed Dec 18, 2019
1 parent f7860b0 commit 3c5ecc2
Show file tree
Hide file tree
Showing 16 changed files with 1,023 additions and 68 deletions.
46 changes: 37 additions & 9 deletions src/compiler/checker.ts
View file
Expand Up @@ -5838,6 +5838,8 @@ namespace ts {
initializer: Expression | undefined
) => T, methodKind: SyntaxKind, useAccessors: boolean): (p: Symbol, isStatic: boolean, baseType: Type | undefined) => (T | AccessorDeclaration | (T | AccessorDeclaration)[]) {
return function serializePropertySymbol(p: Symbol, isStatic: boolean, baseType: Type | undefined) {
const modifierFlags = getDeclarationModifierFlagsFromSymbol(p);
const isPrivate = !!(modifierFlags & ModifierFlags.Private);
if (isStatic && (p.flags & (SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias))) {
// Only value-only-meaning symbols can be correctly encoded as class statics, type/namespace/alias meaning symbols
// need to be merged namespace members
Expand All @@ -5849,34 +5851,35 @@ namespace ts {
&& isTypeIdenticalTo(getTypeOfSymbol(p), getTypeOfPropertyOfType(baseType, p.escapedName)!))) {
return [];
}
const staticFlag = isStatic ? ModifierFlags.Static : 0;
const flag = modifierFlags | (isStatic ? ModifierFlags.Static : 0);
const name = getPropertyNameNodeForSymbol(p, context);
const firstPropertyLikeDecl = find(p.declarations, or(isPropertyDeclaration, isAccessor, isVariableDeclaration, isPropertySignature, isBinaryExpression, isPropertyAccessExpression));
if (p.flags & SymbolFlags.Accessor && useAccessors) {
const result: AccessorDeclaration[] = [];
if (p.flags & SymbolFlags.SetAccessor) {
result.push(setTextRange(createSetAccessor(
/*decorators*/ undefined,
createModifiersFromModifierFlags(staticFlag),
createModifiersFromModifierFlags(flag),
name,
[createParameter(
/*decorators*/ undefined,
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
"arg",
/*questionToken*/ undefined,
serializeTypeForDeclaration(getTypeOfSymbol(p), p)
isPrivate ? undefined : serializeTypeForDeclaration(getTypeOfSymbol(p), p)
)],
/*body*/ undefined
), find(p.declarations, isSetAccessor) || firstPropertyLikeDecl));
}
if (p.flags & SymbolFlags.GetAccessor) {
const isPrivate = modifierFlags & ModifierFlags.Private;
result.push(setTextRange(createGetAccessor(
/*decorators*/ undefined,
createModifiersFromModifierFlags(staticFlag),
createModifiersFromModifierFlags(flag),
name,
[],
serializeTypeForDeclaration(getTypeOfSymbol(p), p),
isPrivate ? undefined : serializeTypeForDeclaration(getTypeOfSymbol(p), p),
/*body*/ undefined
), find(p.declarations, isGetAccessor) || firstPropertyLikeDecl));
}
Expand All @@ -5887,10 +5890,10 @@ namespace ts {
else if (p.flags & (SymbolFlags.Property | SymbolFlags.Variable)) {
return setTextRange(createProperty(
/*decorators*/ undefined,
createModifiersFromModifierFlags((isReadonlySymbol(p) ? ModifierFlags.Readonly : 0) | staticFlag),
createModifiersFromModifierFlags((isReadonlySymbol(p) ? ModifierFlags.Readonly : 0) | flag),
name,
p.flags & SymbolFlags.Optional ? createToken(SyntaxKind.QuestionToken) : undefined,
serializeTypeForDeclaration(getTypeOfSymbol(p), p),
isPrivate ? undefined : serializeTypeForDeclaration(getTypeOfSymbol(p), p),
// TODO: https://github.com/microsoft/TypeScript/pull/32372#discussion_r328386357
// interface members can't have initializers, however class members _can_
/*initializer*/ undefined
Expand All @@ -5899,13 +5902,24 @@ namespace ts {
if (p.flags & (SymbolFlags.Method | SymbolFlags.Function)) {
const type = getTypeOfSymbol(p);
const signatures = getSignaturesOfType(type, SignatureKind.Call);
if (flag & ModifierFlags.Private) {
return setTextRange(createProperty(
/*decorators*/ undefined,
createModifiersFromModifierFlags((isReadonlySymbol(p) ? ModifierFlags.Readonly : 0) | flag),
name,
p.flags & SymbolFlags.Optional ? createToken(SyntaxKind.QuestionToken) : undefined,
/*type*/ undefined,
/*initializer*/ undefined
), find(p.declarations, isFunctionLikeDeclaration) || signatures[0] && signatures[0].declaration || p.declarations[0]);
}

const results = [];
for (const sig of signatures) {
// Each overload becomes a separate method declaration, in order
const decl = signatureToSignatureDeclarationHelper(sig, methodKind, context) as MethodDeclaration;
decl.name = name; // TODO: Clone
if (staticFlag) {
decl.modifiers = createNodeArray(createModifiersFromModifierFlags(staticFlag));
if (flag) {
decl.modifiers = createNodeArray(createModifiersFromModifierFlags(flag));
}
if (p.flags & SymbolFlags.Optional) {
decl.questionToken = createToken(SyntaxKind.QuestionToken);
Expand Down Expand Up @@ -6093,6 +6107,20 @@ namespace ts {
}
}
}
let privateProtected: ModifierFlags = 0;
for (const s of signatures) {
if (s.declaration) {
privateProtected |= getSelectedModifierFlags(s.declaration, ModifierFlags.Private | ModifierFlags.Protected);
}
}
if (privateProtected) {
return [setTextRange(createConstructor(
/*decorators*/ undefined,
createModifiersFromModifierFlags(privateProtected),
/*parameters*/ [],
/*body*/ undefined,
), signatures[0].declaration)];
}
}

const results = [];
Expand Down
39 changes: 28 additions & 11 deletions src/compiler/parser.ts
View file
Expand Up @@ -506,6 +506,9 @@ namespace ts {
return forEach((node as JSDocTypeLiteral).jsDocPropertyTags, cbNode);
case SyntaxKind.JSDocTag:
case SyntaxKind.JSDocClassTag:
case SyntaxKind.JSDocPublicTag:
case SyntaxKind.JSDocPrivateTag:
case SyntaxKind.JSDocProtectedTag:
return visitNode(cbNode, (node as JSDocTag).tagName);
case SyntaxKind.PartiallyEmittedExpression:
return visitNode(cbNode, (<PartiallyEmittedExpression>node).expression);
Expand Down Expand Up @@ -2581,7 +2584,7 @@ namespace ts {
// FormalParameter [Yield,Await]:
// BindingElement[?Yield,?Await]
node.name = parseIdentifierOrPattern();
if (getFullWidth(node.name) === 0 && !hasModifiers(node) && isModifierKind(token())) {
if (getFullWidth(node.name) === 0 && !node.modifiers && isModifierKind(token())) {
// in cases like
// 'use strict'
// function foo(static)
Expand Down Expand Up @@ -3600,7 +3603,7 @@ namespace ts {
return undefined;
}

const isAsync = hasModifier(arrowFunction, ModifierFlags.Async);
const isAsync = hasModifierOfKind(arrowFunction, SyntaxKind.AsyncKeyword);

// If we have an arrow, then try to parse the body. Even if not, try to parse if we
// have an opening brace, just in case we're in an error state.
Expand Down Expand Up @@ -3806,7 +3809,7 @@ namespace ts {
function parseParenthesizedArrowFunctionExpressionHead(allowAmbiguity: boolean): ArrowFunction | undefined {
const node = <ArrowFunction>createNodeWithJSDoc(SyntaxKind.ArrowFunction);
node.modifiers = parseModifiersForArrowFunction();
const isAsync = hasModifier(node, ModifierFlags.Async) ? SignatureFlags.Await : SignatureFlags.None;
const isAsync = hasModifierOfKind(node, SyntaxKind.AsyncKeyword) ? SignatureFlags.Await : SignatureFlags.None;
// Arrow functions are never generators.
//
// If we're speculatively parsing a signature for a parenthesized arrow function, then
Expand Down Expand Up @@ -5002,7 +5005,7 @@ namespace ts {
node.asteriskToken = parseOptionalToken(SyntaxKind.AsteriskToken);

const isGenerator = node.asteriskToken ? SignatureFlags.Yield : SignatureFlags.None;
const isAsync = hasModifier(node, ModifierFlags.Async) ? SignatureFlags.Await : SignatureFlags.None;
const isAsync = hasModifierOfKind(node, SyntaxKind.AsyncKeyword) ? SignatureFlags.Await : SignatureFlags.None;
node.name =
isGenerator && isAsync ? doInYieldAndAwaitContext(parseOptionalIdentifier) :
isGenerator ? doInYieldContext(parseOptionalIdentifier) :
Expand Down Expand Up @@ -5831,9 +5834,9 @@ namespace ts {
node.kind = SyntaxKind.FunctionDeclaration;
parseExpected(SyntaxKind.FunctionKeyword);
node.asteriskToken = parseOptionalToken(SyntaxKind.AsteriskToken);
node.name = hasModifier(node, ModifierFlags.Default) ? parseOptionalIdentifier() : parseIdentifier();
node.name = hasModifierOfKind(node, SyntaxKind.DefaultKeyword) ? parseOptionalIdentifier() : parseIdentifier();
const isGenerator = node.asteriskToken ? SignatureFlags.Yield : SignatureFlags.None;
const isAsync = hasModifier(node, ModifierFlags.Async) ? SignatureFlags.Await : SignatureFlags.None;
const isAsync = hasModifierOfKind(node, SyntaxKind.AsyncKeyword) ? SignatureFlags.Await : SignatureFlags.None;
fillSignature(SyntaxKind.ColonToken, isGenerator | isAsync, node);
node.body = parseFunctionBlockOrSemicolon(isGenerator | isAsync, Diagnostics.or_expected);
return finishNode(node);
Expand Down Expand Up @@ -5866,7 +5869,7 @@ namespace ts {
node.kind = SyntaxKind.MethodDeclaration;
node.asteriskToken = asteriskToken;
const isGenerator = asteriskToken ? SignatureFlags.Yield : SignatureFlags.None;
const isAsync = hasModifier(node, ModifierFlags.Async) ? SignatureFlags.Await : SignatureFlags.None;
const isAsync = hasModifierOfKind(node, SyntaxKind.AsyncKeyword) ? SignatureFlags.Await : SignatureFlags.None;
fillSignature(SyntaxKind.ColonToken, isGenerator | isAsync, node);
node.body = parseFunctionBlockOrSemicolon(isGenerator | isAsync, diagnosticMessage);
return finishNode(node);
Expand Down Expand Up @@ -6508,7 +6511,7 @@ namespace ts {
}

function isAnExternalModuleIndicatorNode(node: Node) {
return hasModifier(node, ModifierFlags.Export)
return hasModifierOfKind(node, SyntaxKind.ExportKeyword)
|| node.kind === SyntaxKind.ImportEqualsDeclaration && (<ImportEqualsDeclaration>node).moduleReference.kind === SyntaxKind.ExternalModuleReference
|| node.kind === SyntaxKind.ImportDeclaration
|| node.kind === SyntaxKind.ExportAssignment
Expand All @@ -6525,6 +6528,11 @@ namespace ts {
return isImportMeta(node) ? node : forEachChild(node, walkTreeForExternalModuleIndicators);
}

/** Do not use hasModifier inside the parser; it relies on parent pointers. Use this instead. */
function hasModifierOfKind(node: Node, kind: SyntaxKind) {
return some(node.modifiers, m => m.kind === kind);
}

function isImportMeta(node: Node): boolean {
return isMetaProperty(node) && node.keywordToken === SyntaxKind.ImportKeyword && node.name.escapedText === "meta";
}
Expand Down Expand Up @@ -6826,7 +6834,16 @@ namespace ts {
break;
case "class":
case "constructor":
tag = parseClassTag(start, tagName);
tag = parseSimpleTag(start, SyntaxKind.JSDocClassTag, tagName);
break;
case "public":
tag = parseSimpleTag(start, SyntaxKind.JSDocPublicTag, tagName);
break;
case "private":
tag = parseSimpleTag(start, SyntaxKind.JSDocPrivateTag, tagName);
break;
case "protected":
tag = parseSimpleTag(start, SyntaxKind.JSDocProtectedTag, tagName);
break;
case "this":
tag = parseThisTag(start, tagName);
Expand Down Expand Up @@ -7192,8 +7209,8 @@ namespace ts {
return node;
}

function parseClassTag(start: number, tagName: Identifier): JSDocClassTag {
const tag = <JSDocClassTag>createNode(SyntaxKind.JSDocClassTag, start);
function parseSimpleTag(start: number, kind: SyntaxKind, tagName: Identifier): JSDocTag {
const tag = <JSDocTag>createNode(kind, start);
tag.tagName = tagName;
return finishNode(tag);
}
Expand Down
15 changes: 6 additions & 9 deletions src/compiler/transformers/declarations.ts
View file
Expand Up @@ -840,13 +840,11 @@ namespace ts {
ensureType(input, input.type)
));
case SyntaxKind.Constructor: {
const isPrivate = hasModifier(input, ModifierFlags.Private);
// A constructor declaration may not have a type annotation
const ctor = createSignatureDeclaration(
SyntaxKind.Constructor,
isPrivate ? undefined : ensureTypeParams(input, input.typeParameters),
// TODO: GH#18217
isPrivate ? undefined! : updateParamsList(input, input.parameters, ModifierFlags.None),
ensureTypeParams(input, input.typeParameters),
updateParamsList(input, input.parameters, ModifierFlags.None),
/*type*/ undefined
);
ctor.modifiers = createNodeArray(ensureModifiers(input));
Expand All @@ -865,15 +863,14 @@ namespace ts {
return cleanup(sig);
}
case SyntaxKind.GetAccessor: {
const isPrivate = hasModifier(input, ModifierFlags.Private);
const accessorType = getTypeAnnotationFromAllAccessorDeclarations(input, resolver.getAllAccessorDeclarations(input));
return cleanup(updateGetAccessor(
input,
/*decorators*/ undefined,
ensureModifiers(input),
input.name,
updateAccessorParamsList(input, isPrivate),
!isPrivate ? ensureType(input, accessorType) : undefined,
updateAccessorParamsList(input, hasModifier(input, ModifierFlags.Private)),
ensureType(input, accessorType),
/*body*/ undefined));
}
case SyntaxKind.SetAccessor: {
Expand All @@ -892,7 +889,7 @@ namespace ts {
ensureModifiers(input),
input.name,
input.questionToken,
!hasModifier(input, ModifierFlags.Private) ? ensureType(input, input.type) : undefined,
ensureType(input, input.type),
ensureNoInitializer(input)
));
case SyntaxKind.PropertySignature:
Expand All @@ -901,7 +898,7 @@ namespace ts {
ensureModifiers(input),
input.name,
input.questionToken,
!hasModifier(input, ModifierFlags.Private) ? ensureType(input, input.type) : undefined,
ensureType(input, input.type),
ensureNoInitializer(input)
));
case SyntaxKind.MethodSignature: {
Expand Down
15 changes: 15 additions & 0 deletions src/compiler/types.ts
View file
Expand Up @@ -468,6 +468,9 @@ namespace ts {
JSDocAugmentsTag,
JSDocAuthorTag,
JSDocClassTag,
JSDocPublicTag,
JSDocPrivateTag,
JSDocProtectedTag,
JSDocCallbackTag,
JSDocEnumTag,
JSDocParameterTag,
Expand Down Expand Up @@ -2617,6 +2620,18 @@ namespace ts {
kind: SyntaxKind.JSDocClassTag;
}

export interface JSDocPublicTag extends JSDocTag {
kind: SyntaxKind.JSDocPublicTag;
}

export interface JSDocPrivateTag extends JSDocTag {
kind: SyntaxKind.JSDocPrivateTag;
}

export interface JSDocProtectedTag extends JSDocTag {
kind: SyntaxKind.JSDocProtectedTag;
}

export interface JSDocEnumTag extends JSDocTag, Declaration {
parent: JSDoc;
kind: SyntaxKind.JSDocEnumTag;
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/utilities.ts
View file
Expand Up @@ -4125,6 +4125,15 @@ namespace ts {
}
}

if (isInJSFile(node) && !!node.parent) {
// getModifierFlagsNoCache should only be called when parent pointers are set,
// or when !(node.flags & NodeFlags.Synthesized) && node.kind !== SyntaxKind.SourceFile)
const tags = (getJSDocPublicTag(node) ? ModifierFlags.Public : ModifierFlags.None)
| (getJSDocPrivateTag(node) ? ModifierFlags.Private : ModifierFlags.None)
| (getJSDocProtectedTag(node) ? ModifierFlags.Protected : ModifierFlags.None);
flags |= tags;
}

if (node.flags & NodeFlags.NestedNamespace || (node.kind === SyntaxKind.Identifier && (<Identifier>node).isInJSDocNamespace)) {
flags |= ModifierFlags.Export;
}
Expand Down

0 comments on commit 3c5ecc2

Please sign in to comment.