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.

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

avoid_returning_this false positive? #3853

Closed
jakemac53 opened this issue Nov 17, 2022 · 1 comment · Fixed by #4339
Closed

avoid_returning_this false positive? #3853

jakemac53 opened this issue Nov 17, 2022 · 1 comment · Fixed by #4339
Labels
false-positive P2 A bug or feature request we're likely to work on

Comments

@jakemac53
Copy link
Contributor

jakemac53 commented Nov 17, 2022

Describe the issue

On the latest dev (2.19.0-398.0.dev) we started seeing this lint trigger for the following code:

BuilderOptions overrideWith(BuilderOptions? other) {
  if (other == null) return this;
  return BuilderOptions(
      {}
        ..addAll(config)
        ..addAll(other.config),
      isRoot: other.isRoot);
}

This is not simply returning this just to have a "fluid interface" - it also might return a new instance. So I don't believe the lint is intended to trigger in this case?

To Reproduce

Check out the build repo (https://github.com/dart-lang/build) at this commit e4d6b8d8d5ed01752c6726e2dcc324a1d60d8d6d, on the latest dev sdk, and run analyzer in the build directory.

Expected behavior

I don't expect to see any lints.

Additional context
Add any other context about the problem here.

@pq pq added type-question A question about expected behavior or functionality P2 A bug or feature request we're likely to work on labels Nov 17, 2022
@lrhn
Copy link
Member

lrhn commented Nov 18, 2022

Agree. The argument for not returning this is that you should use cascades instead of chaining. That only makes sense if the return this; is unconditional, otherwise chaining won't be the same as cascades anyway.

@pq pq added false-positive and removed type-question A question about expected behavior or functionality labels Nov 18, 2022
@pq pq closed this as completed in #4339 May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive P2 A bug or feature request we're likely to work on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants