Skip to content

Conversation

matthewnitschke-wk
Copy link
Contributor

@matthewnitschke-wk matthewnitschke-wk commented Mar 6, 2023

FEA-1506

Issue Status

Closes #14
Closes #15

Also reworked scip-visitor a bit, to abstract the process of creating a "definition" and a "reference" better

@rmconsole-wf
Copy link

rmconsole-wf commented Mar 6, 2023

Merge Requirements Met ✅

Request Rosie to automerge this pull request by including @Workiva/release-management-p in a comment.

General Information

Ticket(s):

Code Review(s): #16
Release Image Tags:

  • drydock.workiva.net/workiva/scip-dart:4356615
  • drydock.workiva.net/workiva/scip-dart:pr-16

Reviewers: corwinsheahan-wf, matthewnitschke-wk

Additional Information

Watchlist Notifications: None

	When this pull is merged I will add it to the following release:
	Version: scip-dart 1.0.4
	Release Ticket(s): RM-183271


Note: This is a shortened report. Click here to view Rosie's full evaluation.
Last updated on Saturday, March 18 04:33 PM CST

@matthewnitschke-wk matthewnitschke-wk changed the title Fixed some issues with declaration vs references Fixed indexing on field parameters and import prefixes Mar 6, 2023
@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.

// ^^^ definition scip-dart pub dart_test 1.0.0 lib/other.dart/Foo#<constructor>().
// documentation ```dart
// ^^^ reference scip-dart pub dart_test 1.0.0 lib/other.dart/Foo#
// ^^^^ reference local 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

named field parameters are now indexed, this is a local reference because its private

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

Choose a reason for hiding this comment

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

Main fix in this pr, instead of skipping field parameters, they are indexed

These are not private, so they are not considered to be local

Comment on lines +1 to +4
import 'dart:math' as math;
// definition scip-dart pub dart_test 1.0.0 lib/more.dart/
// ^^^^ definition scip-dart pub dart_test 1.0.0 lib/more.dart/math.
// documentation ```dart
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as math was added to demonstrate the as indexing fix

Before, math was considered a reference, but it is actually a definition

Comment on lines +59 to +60
} else if (node is NormalFormalParameter) {
_visitNormalFormalParameter(node);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NormalFormalParameter both field and regular parameters

@matthewnitschke-wk matthewnitschke-wk marked this pull request as ready for review March 6, 2023 18:08
@bender-wk bender-wk changed the title Fixed indexing on field parameters and import prefixes FEA-1506: Fixed indexing on field parameters and import prefixes Mar 6, 2023
@matthewnitschke-wk matthewnitschke-wk requested a review from a team March 6, 2023 18:09

@override
void visitNode(AstNode node) {
// print(':: $node ${node.runtimeType}');

Choose a reason for hiding this comment

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

#nit leftover print

@matthewnitschke-wk
Copy link
Contributor Author

QA +1

Tests pass

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

Index this.x constructor parameters as declaration in directives treated as reference
5 participants