Skip to content
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.

Sign up for GitHub

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-2227: Fixed logic for constructor references #165

Merged
merged 7 commits into from
Jan 3, 2025
Merged

FEDX-2227: Fixed logic for constructor references #165

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c052dc
fixed logic for constructor references
matthewnitschke-wk Dec 19, 2024
cfb8388
removed print
matthewnitschke-wk Dec 19, 2024
169275a
fixed bug where all constructor parameters stopped indexing
matthewnitschke-wk Dec 19, 2024
d29d685
fixed constructor declarations in prefixed identifiers
matthewnitschke-wk Dec 19, 2024
f003e51
fixed named constructors
matthewnitschke-wk Dec 19, 2024
3c62e21
fmt
matthewnitschke-wk Dec 19, 2024
69bb98d
documentation updates
matthewnitschke-wk Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ gen-proto:
protoc --dart_out=. ./lib/src/gen/scip.proto

print:
scip print ./index.scip
scip print ./index.scip

print-ast:
dart run ./tool/ast_printer.dart ./snapshots/input/staging-project
32 changes: 32 additions & 0 deletions lib/src/symbol_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,38 @@ class SymbolGenerator {
} else if (node is SimpleIdentifier) {
var element = node.staticElement;

// 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a missing end of sentence here?

if (node.parent is ConstructorDeclaration) {
return null;
}

// if we're nested under a ConstructorName identifier, use the constructor
// as the element to annotate instead of the reference to the Class
final parentConstructor = node.thisOrAncestorOfType<ConstructorName>();
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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling nit: 'fo'

final parentPrefixIdentifier =
node.thisOrAncestorOfType<PrefixedIdentifier>();
if (parentPrefixIdentifier?.prefix == node) return element;

// Constructors can be named: `Foo.bar()`, both `Foo` and `bar` are SimpleIdentifiers
// When the constructor is named, 'bar' is the constructor reference and `Foo` should
// reference the class
if (parentConstructor.name == node) {
return parentConstructor.staticElement;
} else if (parentConstructor.name != null) {
return element;
}

// Otherwise, constructor is just `Foo()`, so simply return the
// constructor's element
return parentConstructor.staticElement;
}

// Both `.loadLibrary()`, and `.call()` are synthetic functions that
// have no definition. These should therefore should not be indexed.
if (element is FunctionElement && element.isSynthetic) {
Expand Down
13 changes: 9 additions & 4 deletions snapshots/input/basic-project/lib/more.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class Animal with SleepMixin {
}
}

factory Animal.cat() => Animal('Timmy', type: AnimalType.cat);

void makeSound() {
soundMaker?.call();
}
Expand All @@ -54,16 +56,19 @@ void main() {
List<int> numbers = [1, 2, 3, 4, 5];
int sum = calculateSum(numbers);

Animal cat = Animal('Kitty', type: AnimalType.cat);
Animal bird = Animal('Kitty', type: AnimalType.bird);
Animal dog = Animal('Buddy', type: AnimalType.dog);
Animal cat = Animal.cat();

cat.makeSound();
cat.sleep();
bird.makeSound();
bird.sleep();

dog.makeSound();
dog.sleep();

print(cat);
cat.makeSound();

print(bird);
print(dog);
print('The sum of $numbers is $sum');

Expand Down
2 changes: 1 addition & 1 deletion snapshots/input/basic-project/lib/relationships.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ class Dog extends Animal with SwimAction {
String get someGetter => 'value';

@override
set someSetter(String v) {};
set someSetter(String v) => print(v);
}
2 changes: 1 addition & 1 deletion snapshots/output/basic-project/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
// ^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`int.dart`/int#
// ^^^^^ 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>`().
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we're correctly referencing <constructor>, fixed by: https://github.com/Workiva/scip-dart/pull/165/files#diff-0ab533d80a9e11da530798d9b92364a97087c4754ab8251de00d85b635c6a4e4R76

// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value)
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value2)
}
61 changes: 38 additions & 23 deletions snapshots/output/basic-project/lib/more.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

Animal(this.name, {required this.type}) {
// ^^^^^^ definition scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#name.
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#type.
// ^^^^ definition scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().(type)
Expand Down Expand Up @@ -77,6 +76,13 @@
}
}

factory Animal.cat() => Animal('Timmy', type: AnimalType.cat);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very frustratingly, in named constructors, Animal.cat() there is only a single identifier

Screenshot 2024-12-19 at 10 57 04 AM

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a ticket to track this here: #166

// ^^^ definition scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#cat().
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().(type)
// ^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#cat.

void makeSound() {
// ^^^^^^^^^ definition scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#makeSound().
soundMaker?.call();
Expand Down Expand Up @@ -122,27 +128,32 @@
// ^^^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/calculateSum().
// ^^^^^^^ reference local 3

Animal cat = Animal('Kitty', type: AnimalType.cat);
Animal bird = Animal('Kitty', type: AnimalType.bird);
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^ definition local 5
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().(type)
// ^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#cat.
// ^^^^ definition local 5
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().(type)
// ^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#bird.
Animal dog = Animal('Buddy', type: AnimalType.dog);
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^ definition local 6
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#`<constructor>`().(type)
// ^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/AnimalType#dog.
Animal cat = Animal.cat();
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^ definition local 7
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#cat().

cat.makeSound();
// ^^^ reference local 5
// ^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#makeSound().
cat.sleep();
// ^^^ reference local 5
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/SleepMixin#sleep().
bird.makeSound();
// ^^^^ reference local 5
// ^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#makeSound().
bird.sleep();
// ^^^^ reference local 5
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/SleepMixin#sleep().

dog.makeSound();
// ^^^ reference local 6
Expand All @@ -151,9 +162,13 @@
// ^^^ reference local 6
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/SleepMixin#sleep().

print(cat);
cat.makeSound();
// ^^^ reference local 7
// ^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`more.dart`/Animal#makeSound().

print(bird);
// ^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`print.dart`/print().
// ^^^ reference local 5
// ^^^^ reference local 5
print(dog);
// ^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`print.dart`/print().
// ^^^ reference local 6
Expand All @@ -165,23 +180,23 @@
print(math.Rectangle(1,2,3,4));
// ^^^^^ 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`/math.
// ^^^^^^^^^ reference scip-dart pub dart:math 2.19.0 dart:math/`rectangle.dart`/Rectangle#
// ^^^^^^^^^ reference scip-dart pub dart:math 2.19.0 dart:math/`rectangle.dart`/Rectangle#`<constructor>`().

[1,2].reduce((a, b) => a + b);
// ^^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`iterable.dart`/Iterable#reduce().
// ^ definition local 7
// ^ definition local 8
// ^ reference local 7
// ^ reference local 8
// ^ definition local 8
// ^ definition local 9
// ^ reference local 8
// ^ reference local 9
}

void test(String Function(int) p) {}
// ^^^^ definition scip-dart pub dart_test 1.0.0 lib/`more.dart`/test().
// ^^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`string.dart`/String#
// ^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`int.dart`/int#
// ^ definition local 9
// ^ definition local 10
void deepTest(String Function(void Function(String test)) p) {}
// ^^^^^^^^ definition scip-dart pub dart_test 1.0.0 lib/`more.dart`/deepTest().
// ^^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`string.dart`/String#
// ^^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`string.dart`/String#
// ^ definition local 10
// ^ definition local 11
8 changes: 3 additions & 5 deletions snapshots/output/basic-project/lib/other.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
// ^^^^^^ definition scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#value3.
Foo(
// ^^^ definition scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#
this._far, {
// ^^^^ reference local 0
required this.value,
Expand All @@ -44,7 +43,6 @@
// ^^^^^^^^^^ definition local 1
Bar(this._someValue);
// ^^^ definition scip-dart pub dart_test 1.0.0 lib/`other.dart`/Bar#`<constructor>`().
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Bar#
// ^^^^^^^^^^ reference local 1

void someMethod() {
Expand All @@ -64,20 +62,20 @@
// ^^^^ reference scip-dart pub dart:async 2.19.0 dart:async/`future.dart`/Future#then().
// ^ definition local 2
Bar('a').someMethod.call()
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Bar#
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Bar#`<constructor>`().
// ^^^^^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Bar#someMethod().
});

Foo(1, value: true, value2: 'asdf')..value = false;
// ^^^ 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>`().
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value)
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value2)
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#value.

final someStr = 'someStr';
// ^^^^^^^ definition local 3
Foo(2, value: false, value2: 'some Val!')
// ^^^ 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>`().
// ^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value)
// ^^^^^^ reference scip-dart pub dart_test 1.0.0 lib/`other.dart`/Foo#`<constructor>`().(value2)
..value = true
Expand Down
4 changes: 3 additions & 1 deletion snapshots/output/basic-project/lib/relationships.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,11 @@

@override
// ^^^^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`annotations.dart`/override.
set someSetter(String v) {};
set someSetter(String v) => print(v);
// ^^^^^^^^^^ definition scip-dart pub dart_test 1.0.0 lib/`relationships.dart`/Dog#`<set>someSetter`.
// relationship scip-dart pub dart_test 1.0.0 lib/`relationships.dart`/Mammal#`<set>someSetter`. implementation reference
// ^^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`string.dart`/String#
// ^ definition local 1
// ^^^^^ reference scip-dart pub dart:core 2.19.0 dart:core/`print.dart`/print().
// ^ reference local 1
}
Loading