-
Notifications
You must be signed in to change notification settings - Fork 650
Add the 5 modifier jsdoc tags #927
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.
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
Changes from 2 commits
1b072fe
1cc2dde
f1968fe
a84616b
451dbb5
a921b63
bb9356b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -105,39 +105,43 @@ func (p *Parser) reparseTags(parent *ast.Node, jsDoc []*ast.Node) { | |||||
} | ||||||
switch tag.Kind { | ||||||
case ast.KindJSDocTypeTag: | ||||||
if parent.Kind == ast.KindVariableStatement && parent.AsVariableStatement().DeclarationList != nil { | ||||||
for _, declaration := range parent.AsVariableStatement().DeclarationList.AsVariableDeclarationList().Declarations.Nodes { | ||||||
if declaration.AsVariableDeclaration().Type == nil { | ||||||
declaration.AsVariableDeclaration().Type = p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, declaration) | ||||||
break | ||||||
switch parent.Kind { | ||||||
case ast.KindVariableStatement: | ||||||
if parent.AsVariableStatement().DeclarationList != nil { | ||||||
for _, declaration := range parent.AsVariableStatement().DeclarationList.AsVariableDeclarationList().Declarations.Nodes { | ||||||
if declaration.AsVariableDeclaration().Type == nil { | ||||||
declaration.AsVariableDeclaration().Type = p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, declaration) | ||||||
break | ||||||
} | ||||||
} | ||||||
} | ||||||
} else if parent.Kind == ast.KindVariableDeclaration { | ||||||
case ast.KindVariableDeclaration: | ||||||
if parent.AsVariableDeclaration().Type == nil { | ||||||
parent.AsVariableDeclaration().Type = p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, parent) | ||||||
} | ||||||
} else if parent.Kind == ast.KindPropertyDeclaration { | ||||||
case ast.KindPropertyDeclaration: | ||||||
declaration := parent.AsPropertyDeclaration() | ||||||
if declaration.Type == nil { | ||||||
declaration.Type = p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, parent) | ||||||
} | ||||||
} else if parent.Kind == ast.KindPropertyAssignment { | ||||||
case ast.KindPropertyAssignment: | ||||||
prop := parent.AsPropertyAssignment() | ||||||
prop.Initializer = p.makeNewTypeAssertion(p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, nil), prop.Initializer) | ||||||
} else if parent.Kind == ast.KindExportAssignment { | ||||||
case ast.KindExportAssignment: | ||||||
export := parent.AsExportAssignment() | ||||||
export.Expression = p.makeNewTypeAssertion(p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, nil), export.Expression) | ||||||
} else if parent.Kind == ast.KindReturnStatement { | ||||||
case ast.KindReturnStatement: | ||||||
ret := parent.AsReturnStatement() | ||||||
ret.Expression = p.makeNewTypeAssertion(p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, nil), ret.Expression) | ||||||
} else if parent.Kind == ast.KindParenthesizedExpression { | ||||||
case ast.KindParenthesizedExpression: | ||||||
paren := parent.AsParenthesizedExpression() | ||||||
paren.Expression = p.makeNewTypeAssertion(p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, nil), paren.Expression) | ||||||
} else if parent.Kind == ast.KindExpressionStatement && | ||||||
parent.AsExpressionStatement().Expression.Kind == ast.KindBinaryExpression { | ||||||
bin := parent.AsExpressionStatement().Expression.AsBinaryExpression() | ||||||
if ast.GetAssignmentDeclarationKind(bin) != ast.JSDeclarationKindNone { | ||||||
bin.Right = p.makeNewTypeAssertion(p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, nil), bin.Right) | ||||||
case ast.KindExpressionStatement: | ||||||
if parent.AsExpressionStatement().Expression.Kind == ast.KindBinaryExpression { | ||||||
bin := parent.AsExpressionStatement().Expression.AsBinaryExpression() | ||||||
if ast.GetAssignmentDeclarationKind(bin) != ast.JSDeclarationKindNone { | ||||||
bin.Right = p.makeNewTypeAssertion(p.makeNewType(tag.AsJSDocTypeTag().TypeExpression, nil), bin.Right) | ||||||
} | ||||||
} | ||||||
} | ||||||
case ast.KindJSDocTemplateTag: | ||||||
|
@@ -178,6 +182,39 @@ func (p *Parser) reparseTags(parent *ast.Node, jsDoc []*ast.Node) { | |||||
fun.FunctionLikeData().Type = p.makeNewType(tag.AsJSDocReturnTag().TypeExpression, fun) | ||||||
} | ||||||
} | ||||||
case ast.KindJSDocReadonlyTag, ast.KindJSDocPrivateTag, ast.KindJSDocPublicTag, ast.KindJSDocProtectedTag, ast.KindJSDocOverrideTag: | ||||||
switch parent.Kind { | ||||||
case ast.KindPropertyDeclaration, ast.KindMethodDeclaration, ast.KindGetAccessor, ast.KindSetAccessor: | ||||||
// !!! BinaryExpression (this.p assignments) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] Consider implementing handling for BinaryExpression parent nodes (e.g.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't implement handling until I decide how to store modifiers for |
||||||
var keyword ast.Kind | ||||||
switch tag.Kind { | ||||||
case ast.KindJSDocReadonlyTag: | ||||||
keyword = ast.KindReadonlyKeyword | ||||||
case ast.KindJSDocPrivateTag: | ||||||
keyword = ast.KindPrivateKeyword | ||||||
case ast.KindJSDocPublicTag: | ||||||
keyword = ast.KindPublicKeyword | ||||||
case ast.KindJSDocProtectedTag: | ||||||
keyword = ast.KindProtectedKeyword | ||||||
case ast.KindJSDocOverrideTag: | ||||||
keyword = ast.KindOverrideKeyword | ||||||
} | ||||||
modifier := p.factory.NewModifier(keyword) | ||||||
modifier.Loc = tag.Loc | ||||||
modifier.Flags = p.contextFlags | ast.NodeFlagsReparsed | ||||||
var nodes []*ast.Node | ||||||
var loc core.TextRange | ||||||
if parent.Modifiers() == nil { | ||||||
nodes = p.nodeSlicePool.NewSlice(1) | ||||||
nodes[0] = modifier | ||||||
loc = tag.Loc | ||||||
} else { | ||||||
nodes = append(parent.Modifiers().Nodes, modifier) | ||||||
loc = parent.Modifiers().Loc | ||||||
} | ||||||
parent.AsMutable().SetModifiers(p.newModifierList(loc, nodes)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this ever happen on any parent kind, or is there a specific subset of nodes that do this? I'm not entirely certain how I feel about the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the 5 node kinds you see here (including BinaryExpression), unless I missed 1 or 2. Would you prefer a specific SetModifiers on those types? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In a way I think it'd be fine to just have a top level ast package function with a switch case in it; just wasn't sure if it was that simple or if the method dispatch here was simplifying things. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the 5 you see here: class members and expando class members. Based on experience, I predict were going to need this to generalise someday, but definitely not today. |
||||||
} | ||||||
// !!! @extends, @implements, @this, @satisfies | ||||||
} | ||||||
} | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
jsdocAccessibilityTag.js(50,14): error TS2341: Property 'priv' is private and only accessible within class 'A'. | ||
jsdocAccessibilityTag.js(58,9): error TS2341: Property 'priv' is private and only accessible within class 'A'. | ||
jsdocAccessibilityTag.js(58,24): error TS2445: Property 'prot' is protected and only accessible within class 'A' and its subclasses. | ||
jsdocAccessibilityTag.js(59,9): error TS2341: Property 'priv' is private and only accessible within class 'A'. | ||
jsdocAccessibilityTag.js(59,24): error TS2445: Property 'prot' is protected and only accessible within class 'A' and its subclasses. | ||
|
||
|
||
==== jsdocAccessibilityTag.js (5 errors) ==== | ||
class A { | ||
/** | ||
* Ap docs | ||
* | ||
* @private | ||
*/ | ||
priv = 4; | ||
/** | ||
* Aq docs | ||
* | ||
* @protected | ||
*/ | ||
prot = 5; | ||
/** | ||
* Ar docs | ||
* | ||
* @public | ||
*/ | ||
pub = 6; | ||
/** @public */ | ||
get ack() { return this.priv } | ||
/** @private */ | ||
set ack(value) { } | ||
} | ||
class C { | ||
constructor() { | ||
/** | ||
* Cp docs | ||
* | ||
* @private | ||
*/ | ||
this.priv2 = 1; | ||
/** | ||
* Cq docs | ||
* | ||
* @protected | ||
*/ | ||
this.prot2 = 2; | ||
/** | ||
* Cr docs | ||
* | ||
* @public | ||
*/ | ||
this.pub2 = 3; | ||
} | ||
h() { return this.priv2 } | ||
} | ||
class B extends A { | ||
m() { | ||
this.priv + this.prot + this.pub | ||
~~~~ | ||
!!! error TS2341: Property 'priv' is private and only accessible within class 'A'. | ||
} | ||
} | ||
class D extends C { | ||
n() { | ||
this.priv2 + this.prot2 + this.pub2 | ||
} | ||
} | ||
new A().priv + new A().prot + new A().pub | ||
~~~~ | ||
!!! error TS2341: Property 'priv' is private and only accessible within class 'A'. | ||
~~~~ | ||
!!! error TS2445: Property 'prot' is protected and only accessible within class 'A' and its subclasses. | ||
new B().priv + new B().prot + new B().pub | ||
~~~~ | ||
!!! error TS2341: Property 'priv' is private and only accessible within class 'A'. | ||
~~~~ | ||
!!! error TS2445: Property 'prot' is protected and only accessible within class 'A' and its subclasses. | ||
new C().priv2 + new C().prot2 + new C().pub2 | ||
new D().priv2 + new D().prot2 + new D().pub2 | ||
|
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.
this is a drive-by reformat. As usual, best reviewed ignoring whitespace.