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 drops comment before comma in collection literal #1584

Closed
munificent opened this issue Oct 23, 2024 · 5 comments · Fixed by #1587
Closed

Formatter drops comment before comma in collection literal #1584

munificent opened this issue Oct 23, 2024 · 5 comments · Fixed by #1587
Labels

Comments

@munificent
Copy link
Member

Here's a minimal repro of an issue found in a larger piece of code. If you format:

var x = [
  1 // Comment 1.
  ,
  2 // Comment 2.
];

It outputs:

var x = [
  1,
  2, // Comment 2.
];

It seems to only have this bug if the first comment is before the comma (which is weird) and there is a second comment.

@munificent munificent added the bug label Oct 23, 2024
@munificent
Copy link
Member Author

Here's another example (sanitized so it can be shown publicly):

final aaaaaaaaaaaAaaaaaaaAaaa = (AaaaaaaaaaaAaaaaaaaAaaa()
  ..aaaAaaaaaaa.aaaAaa([
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa =
          'Aaaaaaa aaaa 1 is aaa 2 aaaaa aaa aaaa aa aaaaaaaaa if aa is aaaa '
              'aaaa aaa aaaaa'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 1111.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.22)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa'), // AA AAAA
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa = 'Aaaaaaa aaaa 2 is a aaaaa aaaa'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 222.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.19)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa'), // AA AAAA
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa = 'Aaaaaaa aaaa 3'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 33.3
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.15)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa'), // AA AAAA
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa = 'Aaaaaaa aaaa 4'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 4
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.11)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa') // AA AAAA
    ,
  ])
  ..aaaAaaaaaaAaaaa.aaaAaa([
    (AaaaaaaaaaaAaaaaaaaAaaaaaaAaaa()
      ..aaaaaaaAaaaAaaa.aaaAaa(['Aaaaaaa', 'Aaaaa', 'Aaaaa'])
      ..aaaaaaaAaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 1111000000.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.22)),
    (AaaaaaaaaaaAaaaaaaaAaaaaaaAaaa()
      ..aaaaaaaAaaaAaaa.aaaAaa(['Aaaaaaa', 'Aaaa', 'Aaaaaa', 'Aaaaaa Aaaaaa'])
      ..aaaaaaaAaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 112000000.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.234)),
    (AaaaaaaaaaaAaaaaaaaAaaaaaaAaaa()
      ..aaaaaaaAaaaAaaa.aaaAaa(['Aaaaaaa', 'Aaaa', 'Aaaaaaa'])
      ..aaaaaaaAaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 82000000.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.54)),
    (AaaaaaaaaaaAaaaaaaaAaaaaaaAaaa()
      ..aaaaaaaAaaaAaaa.aaaAaa([
        'Aaaaaaa',
        'Aaaaaaaaaaa',
        'Aaaa',
        'Aaaaaaaa Aaaa',
        'Aaaa Aaaaaaaa Aaaa',
        'Aaaaaa Aaaa Aaaaaaaa Aaaa',
        'Aaaaaa Aaaa Aaaaaaaa Aaaa Aaa Aaaaaaaa',
      ])
      ..aaaaaaaAaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 82000000.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.54)),
  ]))
  ..aaaAaaaaaaAaaaaa.aaaAaa([
    aaaaAaaaaaaAaaaa(aaaaaaaAaaaa1, aaaaaa: 1111000000.0, aaaaaaaAaaaaa: 0.22),
    aaaaAaaaaaaAaaaa(aaaaaaaAaaaa2, aaaaaa: 112000000.0, aaaaaaaAaaaaa: 0.234),
    aaaaAaaaaaaAaaaa(aaaaaaaAaaaa3, aaaaaa: 82000000.0, aaaaaaaAaaaaa: 0.54),
    aaaaAaaaaaaAaaaa(aaaaaaaAaaaa4, aaaaaa: 82000000.0, aaaaaaaAaaaaa: 0.54),
  ])
  ..aaaaaa();

final aaaaaaaaaaaAaaaaaaaAaaaAaaAaaaaaa = (AaaaaaaaaaaAaaaaaaaAaaa()
  ..aaaAaaaaaaa.aaaAaa([
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa =
          'Aaaaaaa aaaa 1 is aaa 2 aaaaa aaa aaaa aa aaaaaaaaa if aa is aaaa '
              'aaaa aaa aaaaa'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 1111.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.22)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa'), // AA AAAA
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa = 'Aaaaaaa aaaa 2 is a aaaaa aaaa'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 222.0
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.19)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa'), // AA AAAA
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa = 'Aaaaaaa aaaa 3'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 33.3
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.15)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa'), // AA AAAA
    (AaaaaaaaaaaAaaaaaaaAaaaaaa()
      ..aaaaaaaAaaa = 'Aaaaaaa aaaa 4'
      ..aaaaaaaAaaaa = (AaaaaaaaaaaAaaaaaaaAaaaaaAaaaa()
        ..aaaaaAaaaaaAaaaaa = 4
        ..aaaaaAaaaaaAaaaaaaAaaaaa = 0.11)
      ..aaaaaaaAaaaaaaaaAaa =
          'aaaaa://aaa.aaaaaaa.aaa/aaaaaaa/aaaaaaaaaaaa/aaaaaa/aaaaaaa_aaaaaaaaaaaa.aaa') // AA AAAA
    ,
  ]))
  ..aaaaaa();

@munificent
Copy link
Member Author

And another:

void aaaa() {
  aaaa('aaaaaaaa aaaaa aaaa', () {
    final aaaaaAaaa = {
      '/a/11aa9a9aaa', // aaaa
      '/a/047a6a9', // aaa
      '/a/020aa3', // aaaaaa
      '/a/11aa9aaa8_', // aaa
      '/a/02a1aa', // aaaaaaaa
      '/a/0a2aaaa', // aaaaaaaaaaaaaaaaa
      '/a/0a3_aaa', // aaaaaaaa
      '/a/06_a_', // aaa
      '/a/02a_aa', // aaaaa
      '/a/0aaaa8a', // aaaaaaa
      '/a/11a_7aa2aa', // aaaaaaaaaa
      '/a/11aaaaaaaa', // aaaa
      '/a/0a3aa66', // aaaaaaaaa
      '/a/0a3aaa2', // aaaa
      '/a/03aaaa', // aaaaaaaa
      '/a/11aa1a1aaa', // aaaa
      '/a/11aaa2aa0a', // aaaaa
      '/a/11aaa8aa35', // aaaaaaaa
      '/a/06aaaa8', // aaaaaa
      '/a/026aaa', // aaaaa
      '/a/0aaa75a', // aaaaaaaa
      '/a/07aaa', // aaaaaaa
      '/a/0aaaa5a', // aaaaaaa
      '/a/0aa2a_a', // aaa
      '/a/0aaaa7a', // aaa aaaaa
      '/a/121a7aa2', // aaa aaaaa
      '/a/04aaaaa', // aaaaaa aaaaaa
      '/a/0aaaaaa', // aaaaa aaaaaa
      '/a/0154aa', // aaaaa aaaa aaaaa
      '/a/0aaaaa3', // aaaaa
      '/a/03aa4aa', // aa
      '/a/0a_9a3a', // aaaaaa
      '/a/0aaaaa1', // aaaaaaaa
      '/a/11aaaaaaa_', // aaaaaaaa_aaaaaaaa
      '/a/011a8a4a', // aaaa
      '/a/11aa0a444a' // aaaaaa aaaaaaaaa
      ,
    };
    for (final aaa in aaaaaAaaa) {
      aaaaaa(aaaaaaaAaaaAaaaAaa(aaa)).aaAaaAaaa();
    }
  });

  aaaa('aaaaaaa null for aa aaaaaaa aaa', () {
    aaaaaa(aaaaaaaAaaaAaaaAaa('/a/000')).aaAaaa();
  });
}

@munificent
Copy link
Member Author

And another. This one is slightly different because now the comma and comment at after the last element:

void aaaa() {
  aaaaa(() {
    aaaa(() {
      aaaaaa(
        aaaaaaAaaaaaAaaaaaaAaaAaaa(
          aaaAaaaaa: [aaaAaaaa1, aaaAaaaa2, aaaAaaaa3, aaaAaaaa4],
          aaaaaaaaaAaaaaaaaAaa: Aaa.sync(aaaaaaaaaAaaaaaaa),
        ).aaaaaAaaaaaaaaAaaAaaaaa(
          aaa: [
            aaaaAaaaaaaaAa(1),
            aaaaAaaaaaaaAa(2),
            aaaaAaaaaaaaAa(3),
            aaaaAaaaaaaaAa(4),
          ],
          aaaaaaaa: null,
        ).aaaaa,
      )
        ..aaaAaaaaa([
          aaaaa2, // aaaaaaaa
          aaaaa3, // aaaaa <= aaaaa4.aaaaa
          aaaaa4,
          aaaaa1 // aaaa aaaaa aa 3 & 4, aaa aaaaaa
          ,
        ])
        ..aaaAaaaAaaaa(AaaaAaaaAaaaa(aaaaa, aaa))
        ..aaaAaaaaAaaaAaaaAaaaa(AaaaAaaaAaaaa.AAAA_AAAAA);
    });
  });
}

@munificent
Copy link
Member Author

And:

Aaa<AaaaaaaaaaAaaa, Aaaaaa> _aaaaAaaaaa({
  Aaaa<aaa> aaaaaaaAaaaaaaaa = const [
    361872399767151, // Aaaaaa
    28531086733713 // AaaaaAaa
    ,
  ],
}) => {};

@munificent
Copy link
Member Author

This is using inline block comments, but I suspect is the same issue:

final aaaa = {
  // Aaaaa aaaa aaaaaa.
  1 /* % of aaaaaa aaaaaaa */ : 10 /* aaaaaaaaaaa */,
  5 /* % of aaaaaa aaaaaaa */ : 20 /* aaaaaaaaaaa */,
  13 /* % of aaaaaa aaaaaaa */ : 30 /* aaaaaaaaaaa */,
  14 /* % of aaaaaa aaaaaaa */ : 40 /* aaaaaaaaaaa */,
  22 /* % of aaaaaa aaaaaaa */ : 50 /* aaaaaaaaaaa */,
  23 /* % of aaaaaa aaaaaaa */ : 60 /* aaaaaaaaaaa */,
  24 /* % of aaaaaa aaaaaaa */ : 70 /* aaaaaaaaaaa */,
  29 /* % of aaaaaa aaaaaaa */ : 80 /* aaaaaaaaaaa */,
};

munificent added a commit that referenced this issue Oct 24, 2024
A line comment in a collection literal triggers special formatting
where we allow multiple elements in a single line, like:

```
list = [
  // Comment.
  1, 2, 3
  4, 5,
  6,
];
```

The formatter models that by having a DelimitedListBuilder for the
entire collection literal. Then the elements that should be packed onto
a single line are formatted using a separate DelimitedListBuilder for
each line.

In the very rare case where you have a comment after the last element
on a line but *before* the subsequent comma, the comment would get
dropped. This is because the comment was sitting in the inner
DelimitedListBuilder for the line waiting to be put somewhere, but we
then discard that builder to start a new line or when the whole list is
done.

This fixes that. Whenever we're done with a line, we hoist any remaining
comments up to the outer DelimitedListBuilder. That way the comment gets
put after the comma at the end of the line.

Also bump the min SDK constraint to 3.4.0. I don't think dart_style
works with an older version because of analyzer's SDK constraints.

Fix #1584.
munificent added a commit that referenced this issue Oct 24, 2024
…1587)

A line comment in a collection literal triggers special formatting
where we allow multiple elements in a single line, like:

```
list = [
  // Comment.
  1, 2, 3
  4, 5,
  6,
];
```

The formatter models that by having a DelimitedListBuilder for the
entire collection literal. Then the elements that should be packed onto
a single line are formatted using a separate DelimitedListBuilder for
each line.

In the very rare case where you have a comment after the last element
on a line but *before* the subsequent comma, the comment would get
dropped. This is because the comment was sitting in the inner
DelimitedListBuilder for the line waiting to be put somewhere, but we
then discard that builder to start a new line or when the whole list is
done.

This fixes that. Whenever we're done with a line, we hoist any remaining
comments up to the outer DelimitedListBuilder. That way the comment gets
put after the comma at the end of the line.

Also bump the min SDK constraint to 3.4.0. I don't think dart_style
works with an older version because of analyzer's SDK constraints.

Fix #1584.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant