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

Variable redefinition warning #737

Closed
krp opened this issue Dec 12, 2019 · 5 comments
Closed

Variable redefinition warning #737

krp opened this issue Dec 12, 2019 · 5 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@krp
Copy link

krp commented Dec 12, 2019

I just refactored some static variables into instance variables, and in the process of some sloppy copy-pasting, forgot to remove the type in front of the variable names when instantiating them.

e.g. (simplified example)

class MyWidget extends StatefulWidget {
  @override
  _MyWidgetState createState() => _MyWidgetState();
}

class _MyWidgetState extends State<MyWidget> {
  static String _someString = 'hello';

  @override
  Widget build(BuildContext context) {
    print('_someString: $_someString');

    return Scaffold(
      body: Center(child: Text('_someString: $_someString')),
    );
  }
}

became

class MyWidget extends StatefulWidget {
  @override
  _MyWidgetState createState() => _MyWidgetState();
}

class _MyWidgetState extends State<MyWidget> {
  String _someString;

  @override
  void initState() {
    super.initState();
    String _someString = 'hello';
    print('_someString: $_someString');
  }

  @override
  Widget build(BuildContext context) {
    print('_someString: $_someString');

    return Scaffold(
      body: Center(child: Text('_someString: $_someString')),
    );
  }
}

The problem obviously being that String _someString = 'hello'; should be _someString = 'hello';.
This issue compounds if you have a bunch of code you're refactoring, so luckily some assertions elsewhere caught the fact that they were null.
Dart has some cool tooling, and I was thinking it might be helpful to provide a warning in cases like this (perhaps excluding obvious instances like redefinitions of i, j, k, etc).

Should this be a language feature? An analyzer feature? An editor/plugin feature? I was thinking perhaps like the green underline you get when you forget to call super in a method you're overriding.

image

Thoughts?

@krp krp added the feature Proposed language feature that solves one or more problems label Dec 12, 2019
@bwilkerson
Copy link
Member

I don't believe that it would be appropriate for it to be part of the language spec (if that's what you meant by "language feature"), but I do think it would be a reasonable lint.

@srawlins
Copy link
Member

I believe you are referring to dart-lang/sdk#57537.

@krp
Copy link
Author

krp commented Dec 12, 2019

@bwilkerson / @srawlins Thanks for the link to the other discussion. I'm fairly new to Dart (~6 months) and wasn't aware of the separate repo for the linter.

Is avoid_shadowing_type_parameters on http://dart-lang.github.io/linter/lints/ the same as avoid_shadowing that @a14n mentions in his patch in that thread, and if so should it catch my example above?

The above site mentions that avoid_shadowing_type_parameters is part of the pedantic ruleset, so I've configured analysis_options.yaml to use it via the following instructions: https://dart.dev/guides/language/analysis-options#the-analysis-options-file
and https://github.com/dart-lang/pedantic/#using-the-lints but it's not warning on the above example.

The last comment in that issue appears to be from Jan 25, 2018, just coming up on 2 years ago. Should I move the discussion there and close this post?

@bwilkerson
Copy link
Member

Is avoid_shadowing_type_parameters on http://dart-lang.github.io/linter/lints/ the same as avoid_shadowing that @a14n mentions in his patch in that thread, and if so should it catch my example above?

I don't think so. avoid_shadowing_type_parameters only applies to the type parameters used to declare a generic type or function, and your example is related to shadowing fields, so it wouldn't help you. I'm not hugely familiar with avoid_shadowing, but as far as I can tell it looks at other names and ignores type parameters.

Should I move the discussion there and close this post?

That seems like a good plan to me. I think you're asking for the same functionality that that thread is requesting.

@krp
Copy link
Author

krp commented Dec 12, 2019

@bwilkerson Thanks, done! Given the age of the thread I'm not holding my breath. It'd be nice to have though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

3 participants