-
Notifications
You must be signed in to change notification settings - Fork 790
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
Allow generic attributes #14714
Allow generic attributes #14714
Changes from all commits
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 |
---|---|---|
|
@@ -1154,6 +1154,18 @@ type SynAttribute = | |
|
||
/// The syntax range of the attribute | ||
Range: range | ||
kerams marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// The syntax range of the LESS symbol | ||
LessRange: range option | ||
|
||
/// Type arguments | ||
TypeArgs: SynType list | ||
|
||
/// Ranges of the commas between type arguments | ||
CommaRanges: range list | ||
|
||
/// The syntax range of the GREATER symbol | ||
GreaterRange: range option | ||
} | ||
Comment on lines
+1158
to
1169
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 seems to duplicate info from 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. So the question is whether we extract all 4 as type SynTypeArgumentTrivia = {
LessRange: range option
CommaRanges: range list
GreaterRange: range option
}
type SynTypeArgumentList = {
TypeArgs: SynType list
Trivia: SynTypeArgumentTrivia
} |
||
|
||
/// List of attributes enclosed in [< ... >]. | ||
|
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 feels like a
Trivia
node that @nojaf might have opinions on?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.
Yes,
SynType.App
,SynType.LongIdentApp
andSynExpr.TypeApp
use those 3 range fields too.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.
SynType.App, SynType.LongIdentApp and SynExpr.TypeApp
pre-dateSynTrivia
, I think for a new concept it make sense to have these as trivia.It might also make sense to introduce a new type for
TypeName : SynLongIdent
.