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

Change Effective Dart when "bad" code doesn't pass analysis #2580

Closed
kwalrath opened this issue Aug 14, 2020 · 4 comments · Fixed by #2958
Closed

Change Effective Dart when "bad" code doesn't pass analysis #2580

kwalrath opened this issue Aug 14, 2020 · 4 comments · Fixed by #2958
Assignees
Labels
a.effective-dart Relates to the best practices explained in Effective Dart dev.null-safety Relates to transforming or migrating Dart code to sound null safety e1-hours Can complete in < 8 hours of normal, not dedicated, work infra.structure Relates to the tools that create dart.dev p2-medium Necessary but not urgent concern. Resolve when possible. test.general Relates to unit, integration, perf testing

Comments

@kwalrath
Copy link
Contributor

#2575 fixed the build, partly by removing test code that no longer passes analysis. Two occurrences were in Effective Dart:

Can we produce valid "bad" code for both of these rules, or should we alter/delete the rule?

@kwalrath kwalrath added a.effective-dart Relates to the best practices explained in Effective Dart p2-medium Necessary but not urgent concern. Resolve when possible. e1-hours Can complete in < 8 hours of normal, not dedicated, work labels Aug 14, 2020
@kwalrath kwalrath added dev.null-safety Relates to transforming or migrating Dart code to sound null safety test.general Relates to unit, integration, perf testing labels Aug 14, 2020
@johnpryan
Copy link
Contributor

Just a drive-by comment, but in that first example, the good and bad snippets are exactly the same.

@kwalrath
Copy link
Contributor Author

Just a drive-by comment, but in that first example, the good and bad snippets are exactly the same.

Looks like it's been that way since #889. What the heck?

@kwalrath kwalrath added the act.wait-for-customer Needs response from customer label Aug 19, 2020
@munificent
Copy link
Member

munificent commented Aug 20, 2020

Just a drive-by comment, but in that first example, the good and bad snippets are exactly the same.

Not exactly the same. In the good example, highest is declared using num and in the bad example, it's var.

For the second one, I think removing final from both the good and bad examples should fix it:

// Bad:
class Folder {
  final String name;
  List<Document> contents;

  Folder(this.name) : contents = [];
  Folder.temp() : name = 'temporary'; // Oops! Forgot contents.
}

// Good:
class Folder {
  final String name;
  List<Document> contents = [];

  Folder(this.name);
  Folder.temp() : name = 'temporary';
}

@kwalrath
Copy link
Contributor Author

Just a drive-by comment, but in that first example, the good and bad snippets are exactly the same.

Not exactly the same. In the good example, highest is declared using num and in the bad example, it's var.

Oh wow, that's hard to see! I'm going to add a comment or highlight. It looks like the "bad" class is also missing from the bad snippet.

For the second one, I think removing final from both the good and bad examples should fix it

Cool, thanks!

@kwalrath kwalrath assigned kwalrath and unassigned munificent Aug 21, 2020
@kwalrath kwalrath removed the act.wait-for-customer Needs response from customer label Aug 21, 2020
@kwalrath kwalrath added this to the Null safety: stable release milestone Oct 26, 2020
@kwalrath kwalrath added the infra.structure Relates to the tools that create dart.dev label Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a.effective-dart Relates to the best practices explained in Effective Dart dev.null-safety Relates to transforming or migrating Dart code to sound null safety e1-hours Can complete in < 8 hours of normal, not dedicated, work infra.structure Relates to the tools that create dart.dev p2-medium Necessary but not urgent concern. Resolve when possible. test.general Relates to unit, integration, perf testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants