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

Formatter should handle asserts in initializer list. #522

Closed
lrhn opened this issue Aug 24, 2016 · 13 comments
Closed

Formatter should handle asserts in initializer list. #522

lrhn opened this issue Aug 24, 2016 · 13 comments

Comments

@lrhn
Copy link
Member

lrhn commented Aug 24, 2016

See dart-lang/sdk#27141 and dart-lang/sdk#27142 for context.

When the analyzer supports the syntax for assert initializers, the formatter needs to handle it gracefully.

@sethladd
Copy link

sethladd commented Jan 6, 2017

Hi! This issue is open, but I think we addressed this. Should we close?

cc @munificent

@sethladd
Copy link

sethladd commented Jan 6, 2017

Or, maybe not:

~/tmp $ dartfmt const_asserts.dart 
Could not format because the source could not be parsed:

line 6, column 7 of const_asserts.dart: Expected an identifier.
    : assert(children != null), assert(!children.any((child) => child == null));
      ^^^^^^
line 6, column 7 of const_asserts.dart: Expected an assignment after the field name.
    : assert(children != null), assert(!children.any((child) => child == null));
      ^^^^^^
line 6, column 7 of const_asserts.dart: Expected an identifier.
    : assert(children != null), assert(!children.any((child) => child == null));
      ^^^^^^
line 6, column 7 of const_asserts.dart: A function body must be provided.
    : assert(children != null), assert(!children.any((child) => child == null));
      ^^^^^^
line 6, column 7 of const_asserts.dart: Expected a class member.
    : assert(children != null), assert(!children.any((child) => child == null));
      ^^^^^^
line 6, column 7 of const_asserts.dart: Unexpected text 'assert'.
    : assert(children != null), assert(!children.any((child) => child == null));
      ^^^^^^
line 6, column 13 of const_asserts.dart: Expected a class member.
    : assert(children != null), assert(!children.any((child) => child == null));
            ^
line 6, column 13 of const_asserts.dart: Unexpected text '('.
    : assert(children != null), assert(!children.any((child) => child == null));
            ^
line 6, column 23 of const_asserts.dart: Expected a class member.
    : assert(children != null), assert(!children.any((child) => child == null));
                      ^^
line 6, column 23 of const_asserts.dart: Expected a class member.
    : assert(children != null), assert(!children.any((child) => child == null));
                      ^^
(23 more errors...)
~/tmp $ dartfmt --assert-initializer const_asserts.dart 
Could not find an option named "assert-initializers".

Usage: dartfmt [-n|-w] [files or directories...]

-h, --help                   Shows usage information.
    --version                Shows version information.
-l, --line-length            Wrap lines longer than this.
                             (defaults to "80")

-i, --indent                 Spaces of leading indentation.
                             (defaults to "0")

    --preserve               Selection to preserve, formatted as "start:length".
-n, --dry-run                Show which files would be modified but make no changes.
    --set-exit-if-changed    Return exit code 1 if there are any formatting changes.
-w, --overwrite              Overwrite input files with formatted output.
-m, --machine                Produce machine-readable JSON output.
    --profile                Display profile times after running.
    --follow-links           Follow links to files and directories.
                             If unset, links will be ignored.

-t, --transform              Unused flag for compability with the old formatter.

I'm using dartfmt 0.2.13, not sure if that's the latest?

@sethladd
Copy link

sethladd commented Jan 6, 2017

Ah, we at least need to do this:

    // Parse it.
    var parser = new Parser(stringSource, errorListener);
    parser.enableAssertInitializer = true;

in dart_formatter.dart

I'm sure there's other code we'll need to write...

@sethladd
Copy link

sethladd commented Jan 6, 2017

Tried the branch. Looks like it works for the simple case:

~/tmp $ dart ~/Code/dart_style/bin/format.dart const_asserts.dart 
class Foo {
  final List children;

  const Foo({this.children}) : assert(children != null);
}

main() {
  new Foo(children: null); // no failure

  const Foo(children: const []); // no failure

  const Foo(children: null); // failure
}

Thanks!

(no CLI argument required)

@sethladd
Copy link

I think this is done, yes?

@goderbauer
Copy link

I just wanted to setup the formatter for the flutter/plugins repo and the formatter is failing to format the following files because of this issue:

/cc @tvolkert

@kevmoo
Copy link
Member

kevmoo commented Jun 19, 2017

Hitting this a lot w/ https://github.com/dart-lang/pana – FYI

@dgrove
Copy link

dgrove commented Jun 19, 2017

@munificent any thoughts on when this could be addressed?

@munificent
Copy link
Member

I'll get on it today.

@kevmoo
Copy link
Member

kevmoo commented Jun 20, 2017

Points for rolling this into the SDK 😁

@munificent
Copy link
Member

Waiting until the changes from @nex3 are done first, then yes.

@goderbauer
Copy link

When is this change supposed to be available in the Dart SDK? The latest version 1.25.0-dev.4.0 doesn't seem to include it.

@munificent
Copy link
Member

Soon. I was going to wait until those other changes are done, but that may take a while, so I'm going to go ahead and pull it in now.

collinjackson pushed a commit to collinjackson/flutterfire-old2 that referenced this issue Jun 24, 2019
collinjackson pushed a commit to firebase/flutterfire that referenced this issue Aug 14, 2019
g123k pushed a commit to g123k/flutter_android_intent_with_parts that referenced this issue Sep 4, 2019
Akachu pushed a commit to Akachu/firebase_auth that referenced this issue Sep 16, 2019
nksteven pushed a commit to nksteven/ntf-firebase_messaging that referenced this issue Apr 8, 2020
henkibro pushed a commit to henkibro/url_launcher that referenced this issue Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants