Skip to content

Conversation

matthewnitschke-wk
Copy link
Contributor

@matthewnitschke-wk matthewnitschke-wk commented Apr 20, 2023

Closes #18

Dart has 2 synthetic functions which have no definition: loadLibrary on deferred imports, and call on all functions. These should be completely omitted from the scip symbol references

Comment on lines +52 to +53
// ^^^^ 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().
Copy link
Contributor Author

Choose a reason for hiding this comment

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

loadLibrary has no scip reference, as it has no definition

Comment on lines +57 to +58
// ^^^ 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().
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similarly, call has no reference, as it has no definition

@aviary2-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

@@ -1 +1 @@
FROM drydock-prod.workiva.net/workiva/dart_build_image:3 as build No newline at end of file
Copy link
Contributor Author

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)

@matthewnitschke-wk matthewnitschke-wk changed the title Skip indexing synthetic functions with no definition Avoid indexing synthetic functions with no definition Apr 20, 2023
@bender-wk bender-wk changed the title Avoid indexing synthetic functions with no definition FEA-1781: Avoid indexing synthetic functions with no definition Apr 20, 2023
@matthewnitschke-wk matthewnitschke-wk marked this pull request as ready for review April 20, 2023 00:21
@matthewnitschke-wk matthewnitschke-wk requested a review from a team April 20, 2023 00:22
var element = node.staticElement;

// Both `.loadLibrary()`, and `.call()` are synthetic functions that
// have no definition. These should therefore should not be indexed.
Copy link
Member

Choose a reason for hiding this comment

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

#nit

Suggested change
// have no definition. These should therefore should not be indexed.
// have no definition. Therefore these should not be indexed.

Comment on lines +91 to +92
FunctionElement.LOAD_LIBRARY_NAME,
FunctionElement.CALL_METHOD_NAME,

Choose a reason for hiding this comment

The 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:

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 LOAD_LIBRARY_NAME

@matthewnitschke-wk
Copy link
Contributor Author

QA +1

Verified results of scip snapshots, all changes are what we expect

@matthewnitschke-wk
Copy link
Contributor Author

🚀 @Workiva/release-management-p 🚢

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deferred import's loadLibrary reference emits lint error
6 participants