-
Notifications
You must be signed in to change notification settings - Fork 2
FEDX-1213: Localized element selection logic to the SymbolGenerator #128
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
FEDX-1213: Localized element selection logic to the SymbolGenerator #128
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
offset: node.name.offset, | ||
length: node.name.length, | ||
); | ||
return; |
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 fixes #120
print(AnimalType.values); | ||
// ^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`print.dart`/print(). | ||
// ^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType# |
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 validates the case in #37, don't index .values
// ^^^^ reference local 0 | ||
required this.value, | ||
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#value. | ||
// ^^^^^ definition scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value) |
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.
named parameters are indexed as both references and definitions, this is due to the case mentioned in #120, where they are goto'd from a declaration side of things
// The values field on enums is synthetic, and has no explicit definition like | ||
// other fields do. Skip indexing for this case. | ||
if (element.enclosingElement is EnumElement && | ||
element.name == 'values') { | ||
return null; | ||
} |
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 the part that closes #37
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.
Just curious: This codebase doesn't appear to have tests. Is there a reason for that?
QA +1
🚀 @Workiva/release-management-p 🚢 |
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.
+1 from RM
There are 2 parts to getting a symbol representation of a dart reference/definition: selecting the correct
Element
based on a providedAstNode
, and building the string using that selected elementBefore this PR, the element selection part was done directly within the scip_visitor, which is not exported from scip-dart. This PR moves all of this logic to the symbol_generator, which is exported and available
The SymbolGenerator was selected as the
elementFor
method is a necessary step in symbol generation. It seemed like a fitting, and convenient place (due to the pre-existing exporting of the class)Also, unrelated, but closes #37 and closes #120