-
Notifications
You must be signed in to change notification settings - Fork 2
FEA-1781: Avoid indexing synthetic functions with no definition #38
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 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 |
---|---|---|
@@ -1 +1 @@ | ||
FROM drydock-prod.workiva.net/workiva/dart_build_image:3 as build | ||
FROM scratch |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -84,6 +84,15 @@ class ScipVisitor extends GeneralizingAstVisitor { | |||||
void _visitSimpleIdentifier(SimpleIdentifier node) { | ||||||
var element = node.staticElement; | ||||||
|
||||||
// Both `.loadLibrary()`, and `.call()` are synthetic functions that | ||||||
// have no definition. These should therefore should not be indexed. | ||||||
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. #nit
Suggested change
|
||||||
if (element is FunctionElement && element.isSynthetic) { | ||||||
if ([ | ||||||
FunctionElement.LOAD_LIBRARY_NAME, | ||||||
FunctionElement.CALL_METHOD_NAME, | ||||||
Comment on lines
+91
to
+92
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 don't suppose the analyzer provides their own list of synthetic functions without definitions? :badpokerface: 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. Unfortunately not I pulled this directly from the analyzer code here: https://sourcegraph.com/github.com/dart-lang/sdk/-/blob/pkg/analyzer/lib/src/dart/analysis/index.dart?L145:27. There's also no references of |
||||||
].contains(element.name)) return; | ||||||
} | ||||||
|
||||||
// [element] for assignment fields is null. If the parent node | ||||||
// is a `CompoundAssignmentExpression`, we know this node is referring | ||||||
// to an assignment line. In that case, use the read/write element attached | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,9 @@ | ||
class Foo { | ||
import 'more.dart' deferred as more; | ||
// definition scip-dart pub dart_test 1.0.0 lib/other.dart/ | ||
// ^^^^ definition scip-dart pub dart_test 1.0.0 lib/other.dart/more. | ||
// documentation ```dart | ||
|
||
class Foo { | ||
// ^^^ definition scip-dart pub dart_test 1.0.0 lib/other.dart/Foo# | ||
// documentation ```dart | ||
int _far; | ||
|
@@ -40,3 +44,17 @@ | |
// ^^^^^^^^^^ reference local 2 | ||
} | ||
} | ||
|
||
void main() { | ||
// ^^^^ definition scip-dart pub dart_test 1.0.0 lib/other.dart/main(). | ||
// documentation ```dart | ||
more.loadLibrary().then((_) => { | ||
// ^^^^ reference scip-dart pub dart_test 1.0.0 lib/other.dart/more. | ||
// ^^^^ reference scip-dart pub dart:async 2.18.0 dart:async/future.dart/Future#then(). | ||
Comment on lines
+52
to
+53
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.
|
||
// ^ definition local 4 | ||
// documentation ```dart | ||
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#someMethod(). | ||
Comment on lines
+57
to
+58
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. Similarly, |
||
}); | ||
} |
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.
leftover from previous release process, now that
scip-dart
is published to pub.dev, publishing will be done manually (until we can figure out how to automate it)