Skip to content

Warning for parameter hiding on ignored parameters #3099

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
DartBot opened this issue May 17, 2012 · 6 comments
Closed

Warning for parameter hiding on ignored parameters #3099

DartBot opened this issue May 17, 2012 · 6 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented May 17, 2012

This issue was originally filed by [email protected]


Here's the setup: I have nested asynchronous methods that each take a callback, and I don't care about the event being passed to the callback. For (potentially contrived) example:

var button = document.query('#some-button');
button.on.click.add((_) { // <-- I don't care about the Event, I'll call it \_
  // Button is clicked
  // ...set up some more UI...
  var anotherButton = document.query('#another-button');
  anotherButton.on.click.add((_) => window.alert('Success!')); // <-- I don't care about this Event either
});

This may be a contrived example, and in retrospect I'm not sure where I got the idea to name things _ when I don't care about them, this might be a personal convention or I may have cargo culted it from Go. If it isn't a style recommendation something like it should be perhaps.

My point is, I currently see a warning when the inner _ hides the outer _. But I don't really care, I don't care about either of them. I can rename the second to __ or something, but this arms race quickly becomes annoying.

It might be nice if Dart considered variables named _ as totally ignored placeholders, not caring if they hide other parameters, and potentially showing warnings/errors if they're ever actually used (e.g., _.size or _.stopPropagation()).

@iposva-google
Copy link
Contributor

Set owner to @gbracha.
Added Area-Language, Accepted labels.

@gbracha
Copy link
Contributor

gbracha commented May 17, 2012

Currently, _ is treated just like any other identifier. We could relax shadowing requirements for it. Or we could relax shadowing requirements altogether.


Removed Type-Defect label.
Added Type-Enhancement label.

@DartBot
Copy link
Author

DartBot commented May 17, 2012

This comment was originally written by [email protected]


Shadowing warnings are often useful and catch bugs. But if there is a standard ignored variable (and I think that's a nice language feature) then it should be exempt from the warning.

As an alternative other languages use unused_foo or similar to mark unused variables, mostly for linting purposes.

@DartBot
Copy link
Author

DartBot commented May 17, 2012

This comment was originally written by [email protected]


(Bah. Wrong account. jasonhall == imjasonh for the record)

Knowing when a variable is unused can also help when minifying JavaScript and Dart since a method can return nothing or return a short constant variable name if it knows the result won't be used.

@gbracha
Copy link
Contributor

gbracha commented May 18, 2012

Actually, we've decided to allow these warnings to be managed by the tools in a less "in your face fashion" so hopefully this will stop being a problem. For example, the IDE could just highlight shadowed identifiers in a special way.

@gbracha
Copy link
Contributor

gbracha commented May 24, 2012

Added WontFix label.

@DartBot DartBot added Type-Enhancement area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels May 24, 2012
@kevmoo kevmoo added closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug and removed resolution-wont_fix labels Mar 1, 2016
@lrhn lrhn unassigned gbracha Feb 4, 2019
copybara-service bot pushed a commit that referenced this issue Jul 29, 2022
…2 revisions)

https://dart.googlesource.com/dartdoc/+log/d3b0b724972f..e476b1a11547

2022-07-29 [email protected] remove the height constraint on the search box (#3100)
2022-07-29 [email protected] Bump analyzer to 4.3.1 (#3099)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/dart-doc-dart-sdk
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Dart Documentation Generator: https://github.com/dart-lang/dartdoc/issues
To file a bug in Dart SDK: https://github.com/dart-lang/sdk/issues

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Tbr: [email protected]
Change-Id: Ic52033c112052bbd2e0eda4f0f2b86786d186e9c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/253041
Commit-Queue: Nate Bosch <[email protected]>
Commit-Queue: DEPS Autoroller <[email protected]>
Reviewed-by: Nate Bosch <[email protected]>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-not-planned Closed as we don't intend to take action on the reported issue type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants