FEDX-2227: Fixed logic for constructor references#165
FEDX-2227: Fixed logic for constructor references#165btr-rmconsole-1[bot] merged 7 commits intomasterfrom
Conversation
| // ^^^^^ definition local 5 | ||
| Foo(1, value: true, value2: ''); | ||
| // ^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo# | ||
| // ^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`(). |
There was a problem hiding this comment.
Now we're correctly referencing <constructor>, fixed by: https://github.com/Workiva/scip-dart/pull/165/files#diff-0ab533d80a9e11da530798d9b92364a97087c4754ab8251de00d85b635c6a4e4R76
| } | ||
| } | ||
|
|
||
| factory Animal.cat() => Animal('Timmy', type: AnimalType.cat); |
There was a problem hiding this comment.
Very frustratingly, in named constructors, Animal.cat() there is only a single identifier
This isn't the case for SimpleIdentifiers for the call sight, but only the constructor declarations
The result of this, is we can only index a single element here, and I've decided to index, the arguably more important, definition for cat()
factory Animal.cat() => ...
// ^^^^^^ NOT INDEXED
// ^^^ definition ... Animal#cat().Solving this is a problem for another day...
There was a problem hiding this comment.
I created a ticket to track this here: #166
bender-wk
changed the title
alanknight-wk
left a comment
alanknight-wk
left a comment
There was a problem hiding this comment.
Two comment nits, otherwise LGTM
lib/src/symbol_generator.dart
Outdated
|
|
||
| // A SimpleIdentifier with a direct parent of a ConstructorDeclaration | ||
| // is the reference to the class itself. In scip, we want to ignore | ||
| // this as the constructor has its own definition, and only that |
There was a problem hiding this comment.
Is there a missing end of sentence here?
lib/src/symbol_generator.dart
Outdated
| if (parentConstructor != null) { | ||
| // ConstructorNames can also include an import PrefixIdentifier: `math.Rectangle()` | ||
| // both 'math' and 'Rectangle' are SimpleIdentifiers. We only want the constructor | ||
| // element fo 'Rectangle' in this case |
|
Going to close and re-open this, trying to fix one of the CI errors. |
|
QA +1
🚀 @Workiva/release-management-p 🚢 |
4 participants
FEDX-2227
Closes #164
Closes #163