Skip to content

Commit bacc9bf

Browse files
authored
Remove tests of invalid import syntax. (#1758)
Remove tests of invalid import syntax. The language specifies that import configuration URIs must appear before any import prefix, like: ```dart import 'foo.dart' if (cond) 'bar.dart' as prefix; ``` If a user wrote them in the wrong order: ```dart import 'foo.dart' as prefix if (cond) 'bar.dart'; ``` Then the parser used to not report an error and would accept the code, which would then be compiled successfully. Some users in the wild, probably inadvertently, ended up relying on this. So I added support in the formatter to handle it. I also filed: dart-lang/sdk#56641 That issue will be fixed soon: dart-lang/sdk#61264 It's going to be a parse error to have the clauses in the wrong order. These formatter tests are failing in the bleeding edge SDK. So now it's time to remove the tests and support for the broken code. (Note that the comment in the tests are backwards. Configuration before prefix is the correct order. It's prefix before configuration that's wrong.)
1 parent b5a166e commit bacc9bf

File tree

7 files changed

+21
-62
lines changed

7 files changed

+21
-62
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
## 3.1.3-wip
22

3+
* No longer format imports with configurations and a prefix in the wrong order.
4+
The parser used to accept this without error even though it violated the
5+
language spec. The parser is being fixed, so the formatter will no longer
6+
accept or format code like:
7+
8+
```dart
9+
import 'foo.dart' as prefix if (cond) 'bar.dart';
10+
```
11+
312
* Don't force a space between `?` and `.` if a null-aware element contains a
413
dot shorthand.
514

lib/src/front_end/piece_factory.dart

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,20 +1016,9 @@ mixin PieceFactory {
10161016
// Add all of the clauses and combinators.
10171017
var clauses = <Piece>[];
10181018

1019-
// The language specifies that configurations must appear after any `as`
1020-
// clause but the parser incorrectly accepts them before it and code in
1021-
// the wild relies on that. Instead of failing with an "unexpected output"
1022-
// error, just preserve the order of the clauses if they are out of order.
1023-
// See: https://github.com/dart-lang/sdk/issues/56641
1024-
var wroteConfigurations = false;
1025-
if (directive.configurations.isNotEmpty &&
1026-
asKeyword != null &&
1027-
directive.configurations.first.ifKeyword.offset < asKeyword.offset) {
1028-
for (var configuration in directive.configurations) {
1029-
clauses.add(nodePiece(configuration));
1030-
}
1031-
1032-
wroteConfigurations = true;
1019+
// Include any `if` clauses.
1020+
for (var configuration in directive.configurations) {
1021+
clauses.add(nodePiece(configuration));
10331022
}
10341023

10351024
// Include the `as` clause.
@@ -1044,13 +1033,6 @@ mixin PieceFactory {
10441033
);
10451034
}
10461035

1047-
// Include any `if` clauses.
1048-
if (!wroteConfigurations) {
1049-
for (var configuration in directive.configurations) {
1050-
clauses.add(nodePiece(configuration));
1051-
}
1052-
}
1053-
10541036
// Include the `show` and `hide` clauses.
10551037
for (var combinatorNode in directive.combinators) {
10561038
switch (combinatorNode) {

lib/src/short/source_visitor.dart

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,18 +1832,7 @@ final class SourceVisitor extends ThrowingAstVisitor {
18321832
space();
18331833
visit(node.uri);
18341834

1835-
// The language specifies that configurations must appear after any `as`
1836-
// clause but the parser incorrectly accepts them before it and code in
1837-
// the wild relies on that. Instead of failing with an "unexpected output"
1838-
// error, just preserve the order of the clauses if they are out of order.
1839-
// See: https://github.com/dart-lang/sdk/issues/56641
1840-
var wroteConfigurations = false;
1841-
if (node.asKeyword case var asKeyword?
1842-
when node.configurations.isNotEmpty &&
1843-
node.configurations.first.ifKeyword.offset < asKeyword.offset) {
1844-
_visitConfigurations(node.configurations);
1845-
wroteConfigurations = true;
1846-
}
1835+
_visitConfigurations(node.configurations);
18471836

18481837
if (node.asKeyword != null) {
18491838
soloSplit();
@@ -1853,10 +1842,6 @@ final class SourceVisitor extends ThrowingAstVisitor {
18531842
visit(node.prefix);
18541843
}
18551844

1856-
if (!wroteConfigurations) {
1857-
_visitConfigurations(node.configurations);
1858-
}
1859-
18601845
_visitCombinators(node.combinators);
18611846
});
18621847
}

test/short/regression/1500/1544.unit

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
>>>
2-
import 'package:archive/archive.dart' as archive
3-
if (dart.library.io) 'package:archive/archive_io.dart';
2+
import 'package:archive/archive.dart'
3+
if (dart.library.io) 'package:archive/archive_io.dart' as archive;
44
import 'package:flutter/services.dart';
55
import 'package:fuzzy/web/e621/e621.dart';
66
import 'package:fuzzy/web/e621/models/tag_d_b.dart';
@@ -47,8 +47,8 @@ final LazyInitializer<TagDB> tagDbLazy = LazyInitializer(() async {
4747
}
4848
});
4949
<<<
50-
import 'package:archive/archive.dart' as archive
51-
if (dart.library.io) 'package:archive/archive_io.dart';
50+
import 'package:archive/archive.dart'
51+
if (dart.library.io) 'package:archive/archive_io.dart' as archive;
5252
import 'package:flutter/services.dart';
5353
import 'package:fuzzy/web/e621/e621.dart';
5454
import 'package:fuzzy/web/e621/models/tag_d_b.dart';

test/short/whitespace/directives.unit

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,6 @@ part of'uri.dart' ;
6767
<<<
6868
part of 'uri.dart';
6969
>>> Both configuration and prefix.
70-
import 'foo.dart' as foo if (config == 'value') 'other.dart';
71-
<<<
72-
import 'foo.dart' as foo
73-
if (config == 'value') 'other.dart';
74-
>>> Configuration before prefix.
75-
### This violates the language spec, but the parser currently allows it without
76-
### reporting an error and code in the wild relies on that. So the formatter
77-
### handles it gracefully. See: https://github.com/dart-lang/sdk/issues/56641
7870
import 'foo.dart' if (config == 'value') 'other.dart' as foo;
7971
<<<
8072
import 'foo.dart'

test/tall/regression/1500/1544.unit

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
>>>
2-
import 'package:archive/archive.dart' as archive
3-
if (dart.library.io) 'package:archive/archive_io.dart';
2+
import 'package:archive/archive.dart'
3+
if (dart.library.io) 'package:archive/archive_io.dart' as archive;
44
import 'package:flutter/services.dart';
55
import 'package:fuzzy/web/e621/e621.dart';
66
import 'package:fuzzy/web/e621/models/tag_d_b.dart';
@@ -48,8 +48,8 @@ final LazyInitializer<TagDB> tagDbLazy = LazyInitializer(() async {
4848
});
4949
<<<
5050
import 'package:archive/archive.dart'
51-
as archive
52-
if (dart.library.io) 'package:archive/archive_io.dart';
51+
if (dart.library.io) 'package:archive/archive_io.dart'
52+
as archive;
5353
import 'package:flutter/services.dart';
5454
import 'package:fuzzy/web/e621/e621.dart';
5555
import 'package:fuzzy/web/e621/models/tag_d_b.dart';

test/tall/top_level/import.unit

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,6 @@ import 'some/uri.dart'
6969
if (config.name.debug ==
7070
'string') 'c';
7171
>>> Both configuration and prefix.
72-
import 'foo.dart' as foo if (config == 'value') 'other.dart';
73-
<<<
74-
import 'foo.dart'
75-
as foo
76-
if (config == 'value') 'other.dart';
77-
>>> Configuration before prefix.
78-
### This violates the language spec, but the parser currently allows it without
79-
### reporting an error and code in the wild relies on that. So the formatter
80-
### handles it gracefully. See: https://github.com/dart-lang/sdk/issues/56641
8172
import 'foo.dart' if (config == 'value') 'other.dart' as foo;
8273
<<<
8374
import 'foo.dart'

0 commit comments

Comments
 (0)