Skip to content

join_return_with_assignment tests should include assignment patterns #59074

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

Closed
srawlins opened this issue Mar 16, 2023 · 4 comments
Closed

join_return_with_assignment tests should include assignment patterns #59074

srawlins opened this issue Mar 16, 2023 · 4 comments
Assignees
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead.
Milestone

Comments

@srawlins
Copy link
Member

Child issue of #58837

I think there should be test cases like:

var (a, b) = record; // variable declaration
return a;

// also
(a, b) = record; // just an assignment
return a;

in which the lint rule would not report that the return should be joined with the assignment.

(There are also no test cases with multiple assignment expressions in one statement, like

  • a = b = 7; return b; or
  • var a = 8, b = 9; return b;

)

@pq pq added this to the Dart 3 beta 3 milestone Mar 16, 2023
@scheglov scheglov self-assigned this Mar 16, 2023
@scheglov
Copy link
Contributor

We don't report this lint even for a single declared local variable.

int f(int a) {
  var a = 0;
  return a;
}

We could, I guess, although this is not an assignment.

@scheglov
Copy link
Contributor

As for multiple assignments, e.g.

int f1(int a, int b) {
  a = b = 0;
  return a;
}

int f2(int a, int b) {
  a = b = 0;
  return b;
}

Semantically it is not wrong to inline as return a = b = 0; in both cases.
But it definitely feels weird.
And we report the lint only for one of them, f1.

@srawlins
Copy link
Member Author

Ah good analysis; we don't need to rock the boat for patterns; if declarations aren't covered, then they aren't covered. No biggie. But the assignment pattern (identifier pattern?) I think should still have a test.

@scheglov
Copy link
Contributor

Tests were added.

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead.
Projects
None yet
Development

No branches or pull requests

4 participants