Skip to content

Commit

Permalink
Avoid crash on EOF error in file with Windows line encoding
Browse files Browse the repository at this point in the history
+ start using spans from CFE (now that they're available)

Change-Id: I4fe82070d32948a64fb0fab527bd4cc857eebeb4
Reviewed-on: https://dart-review.googlesource.com/49841
Reviewed-by: Sigmund Cherem <[email protected]>
  • Loading branch information
johnniwinther committed Apr 9, 2018
1 parent 8054409 commit 969c0de
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 74 deletions.
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*.yaml text

# Files that should not be converted.
tests/compiler/dart2js_extra/eof_line_ending_test.dart -text
tests/compiler/dart2js_extra/string_interpolation_test.dart -text
tests/compiler/dart2js_extra/string_interpolation_dynamic_test.dart -text
tests/compiler/dart2js_extra/literal_string_juxtaposition_test.dart -text
Expand Down
11 changes: 7 additions & 4 deletions pkg/compiler/lib/src/io/source_file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'dart:math';
import 'dart:typed_data' show Uint8List;

import 'package:kernel/ast.dart' as kernel show Location, Source;

import 'location_provider.dart' show LocationProvider;
import '../../compiler_new.dart';

Expand Down Expand Up @@ -102,10 +103,6 @@ abstract class SourceFile<T> implements Input<T>, LocationProvider {
if (colorize == null) {
colorize = (text) => text;
}
if (end > length) {
start = length - 1;
end = length;
}

kernel.Location startLocation = kernelSource.getLocation(null, start);
kernel.Location endLocation = kernelSource.getLocation(null, end);
Expand Down Expand Up @@ -142,9 +139,15 @@ abstract class SourceFile<T> implements Input<T>, LocationProvider {
for (int line = lineStart; line <= lineEnd; line++) {
String textLine = kernelSource.getTextLine(line + 1);
if (line == lineStart) {
if (columnStart > textLine.length) {
columnStart = textLine.length;
}
buf.write(textLine.substring(0, columnStart));
buf.writeln(colorize(textLine.substring(columnStart)));
} else if (line == lineEnd) {
if (columnEnd > textLine.length) {
columnEnd = textLine.length;
}
buf.write(colorize(textLine.substring(0, columnEnd)));
buf.writeln(textLine.substring(columnEnd));
} else {
Expand Down
2 changes: 1 addition & 1 deletion pkg/compiler/lib/src/kernel/front_end_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void reportFrontEndMessage(
Spannable span;
if (message.span != null) {
span = new SourceSpan(message.span.start.sourceUrl,
message.span.start.offset, message.span.start.offset + 1);
message.span.start.offset, message.span.end.offset);
} else {
span = NO_LOCATION_SPANNABLE;
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/compiler/testing.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
],

"exclude": [
"^tests/compiler/dart2js/data/.*",
"^tests/compiler/dart2js/inference/data/super_invoke\\.dart",
"^tests/compiler/dart2js/old_frontend/data/.*",
"^tests/compiler/dart2js/path with spaces/.*"
]
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/compiler/testing_dart.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
"exclude": [
"^tests/compiler/dart2js/codegen_helper\\.dart",
"^tests/compiler/dart2js/codegen/type_inference8_test\\.dart",
"^tests/compiler/dart2js/data/one_line_dart_program\\.dart",
"^tests/compiler/dart2js/deferred/inline_restrictions_test\\.dart",
"^tests/compiler/dart2js/deferred/load_graph_segmentation2_test\\.dart",
"^tests/compiler/dart2js/deferred/load_graph_segmentation_test\\.dart",
Expand Down Expand Up @@ -48,9 +47,10 @@
"^tests/compiler/dart2js/needs_no_such_method_test\\.dart",
"^tests/compiler/dart2js/no_such_method_enabled_test\\.dart",
"^tests/compiler/dart2js/output_collector\\.dart",
"^tests/compiler/dart2js/old_frontend/patch_test\\.dart",
"^tests/compiler/dart2js/old_frontend/data/one_line_dart_program\\.dart",
"^tests/compiler/dart2js/old_frontend/message_kind_helper\\.dart",
"^tests/compiler/dart2js/old_frontend/metadata_test\\.dart",
"^tests/compiler/dart2js/old_frontend/patch_test\\.dart",
"^tests/compiler/dart2js/old_frontend/reexport_handled_test\\.dart",
"^tests/compiler/dart2js/old_frontend/resolution_test\\.dart",
"^tests/compiler/dart2js/quarantined/http_launch_data/http_launch_main_package\\.dart",
Expand Down
44 changes: 0 additions & 44 deletions tests/co19/co19-dart2js.status
Original file line number Diff line number Diff line change
Expand Up @@ -7037,7 +7037,6 @@ Language/Classes/Getters/same_name_method_t05: MissingCompileTimeError
Language/Classes/Getters/same_name_method_t07: MissingCompileTimeError
Language/Classes/Superinterfaces/more_than_once_t01: MissingCompileTimeError
Language/Classes/Superinterfaces/superclass_as_superinterface_t01: MissingCompileTimeError
Language/Classes/definition_t16: Crash
Language/Classes/definition_t24: MissingCompileTimeError
Language/Classes/same_name_instance_and_static_members_t01: MissingCompileTimeError
Language/Classes/same_name_instance_and_static_members_t02: MissingCompileTimeError
Expand All @@ -7060,36 +7059,15 @@ Language/Expressions/Constants/exception_t02: MissingCompileTimeError
Language/Expressions/Constants/logical_expression_t04: MissingCompileTimeError
Language/Expressions/Function_Invocation/Unqualified_Invocation/instance_context_invocation_t04: Crash
Language/Expressions/Instance_Creation/Const/exception_t01: MissingCompileTimeError
Language/Expressions/Maps/syntax_t08: Crash
Language/Expressions/Method_Invocation/Ordinary_Invocation/evaluation_t08: RuntimeError
Language/Expressions/Method_Invocation/Super_Invocation/accessible_instance_member_t02: Crash
Language/Expressions/Method_Invocation/Super_Invocation/evaluation_t05: RuntimeError
Language/Expressions/Method_Invocation/Super_Invocation/invocation_t02: Crash
Language/Expressions/Null/instance_of_class_null_t01: RuntimeError
Language/Expressions/Strings/String_Interpolation/syntax_t08: Crash
Language/Expressions/Strings/multi_line_t11: Crash # Investiage: source-data from FE causes us to crash
Language/Expressions/Strings/multi_line_t12: Crash
Language/Expressions/Strings/multi_line_t13: Crash
Language/Expressions/Strings/multi_line_t15: Crash
Language/Expressions/Strings/multi_line_t16: Crash
Language/Expressions/Strings/multi_line_t17: Crash
Language/Expressions/Strings/multi_line_t18: Crash
Language/Expressions/Strings/multi_line_t19: Crash
Language/Expressions/Strings/multi_line_t20: Crash
Language/Expressions/Strings/multi_line_t21: Crash
Language/Expressions/Strings/multi_line_t22: Crash
Language/Expressions/Strings/multi_line_t23: Crash
Language/Expressions/Strings/multi_line_t24: Crash
Language/Expressions/Strings/multi_line_t25: Crash
Language/Expressions/Strings/multi_line_t28: Crash
Language/Expressions/Strings/multi_line_t29: Crash
Language/Expressions/Strings/multi_line_t32: Crash
Language/Expressions/Strings/multi_line_t33: Crash
Language/Expressions/This/placement_t04: Crash
Language/Functions/External_Functions/not_connected_to_a_body_t01: RuntimeError
Language/Functions/Formal_Parameters/Optional_Formals/default_value_t01: MissingCompileTimeError
Language/Functions/Formal_Parameters/Optional_Formals/default_value_t02: MissingCompileTimeError
Language/Functions/syntax_t09: Crash
Language/Libraries_and_Scripts/Imports/same_name_t10: RuntimeError
Language/Libraries_and_Scripts/Scripts/top_level_main_t01: CompileTimeError
Language/Metadata/before_class_t01: RuntimeError
Expand Down Expand Up @@ -7182,7 +7160,6 @@ Language/Classes/Instance_Methods/Operators/allowed_names_t19: Crash
Language/Classes/Instance_Methods/Operators/allowed_names_t20: Crash
Language/Classes/Instance_Methods/Operators/allowed_names_t21: Crash
Language/Classes/Instance_Methods/Operators/allowed_names_t22: Crash
Language/Classes/definition_t16: Crash
Language/Classes/member_definition_t04: Crash
Language/Classes/member_definition_t06: Crash
Language/Classes/member_definition_t07: Crash
Expand All @@ -7193,39 +7170,18 @@ Language/Classes/member_definition_t11: Crash
Language/Classes/member_definition_t12: Crash
Language/Expressions/Constants/depending_on_itself_t03: Crash
Language/Expressions/Function_Invocation/Unqualified_Invocation/instance_context_invocation_t04: Crash
Language/Expressions/Maps/syntax_t08: Crash
Language/Expressions/Method_Invocation/Super_Invocation/accessible_instance_member_t02: Crash
Language/Expressions/Method_Invocation/Super_Invocation/evaluation_t05: Crash
Language/Expressions/Method_Invocation/Super_Invocation/getter_lookup_failed_t03: Crash
Language/Expressions/Method_Invocation/Super_Invocation/getter_lookup_failed_t04: Crash
Language/Expressions/Method_Invocation/Super_Invocation/invocation_t02: Crash
Language/Expressions/Strings/String_Interpolation/syntax_t08: Crash
Language/Expressions/Strings/multi_line_t11: Crash
Language/Expressions/Strings/multi_line_t12: Crash
Language/Expressions/Strings/multi_line_t13: Crash
Language/Expressions/Strings/multi_line_t15: Crash
Language/Expressions/Strings/multi_line_t16: Crash
Language/Expressions/Strings/multi_line_t17: Crash
Language/Expressions/Strings/multi_line_t18: Crash
Language/Expressions/Strings/multi_line_t19: Crash
Language/Expressions/Strings/multi_line_t20: Crash
Language/Expressions/Strings/multi_line_t21: Crash
Language/Expressions/Strings/multi_line_t22: Crash
Language/Expressions/Strings/multi_line_t23: Crash
Language/Expressions/Strings/multi_line_t24: Crash
Language/Expressions/Strings/multi_line_t25: Crash
Language/Expressions/Strings/multi_line_t28: Crash
Language/Expressions/Strings/multi_line_t29: Crash
Language/Expressions/Strings/multi_line_t32: Crash
Language/Expressions/Strings/multi_line_t33: Crash
Language/Expressions/This/placement_t04: Crash
Language/Functions/setter_modifier_t01: Crash
Language/Functions/setter_modifier_t02: Crash
Language/Functions/setter_modifier_t03: Crash
Language/Functions/setter_modifier_t04: Crash
Language/Functions/setter_modifier_t05: Crash
Language/Functions/setter_modifier_t06: Crash
Language/Functions/syntax_t09: Crash
Language/Statements/For/syntax_t13: Crash
Language/Statements/For/syntax_t20: Crash
LibTest/core/Invocation/namedArguments_A01_t01: Crash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import 'package:compiler/src/commandline_options.dart';
import 'package:expect/expect.dart';
import 'package:path/path.dart' as path;

import 'end_to_end/launch_helper.dart' show launchDart2Js;
import '../end_to_end/launch_helper.dart' show launchDart2Js;

Uri pathOfData = Platform.script;
Directory tempDir;
Expand Down Expand Up @@ -53,30 +53,35 @@ void check(ProcessResult result) {
Expect.notEquals(0, result.exitCode);
List<int> stdout = result.stdout;
String stdoutString = utf8.decode(stdout);
Expect.isTrue(stdoutString.contains("Error"));
Expect.isTrue(stdoutString.contains("Error"), "stdout:\n$stdoutString");
// Make sure the "499" from the last line is in the output.
Expect.isTrue(stdoutString.contains("499"));
Expect.isTrue(stdoutString.contains("499"), "stdout:\n$stdoutString");

// Make sure that the output does not contain any 0 character.
Expect.isFalse(stdout.contains(0));
}

Future testFile({bool useKernel}) async {
Future testFile() async {
String inFilePath =
pathOfData.resolve('data/one_line_dart_program.dart').path;
List<String> args = [inFilePath, "--out=" + outFilePath];
if (!useKernel) args.add(Flags.useOldFrontend);

List<String> args = [
inFilePath,
"--out=" + outFilePath,
Flags.useOldFrontend
];
await cleanup();
check(await launchDart2Js(args, noStdoutEncoding: true));
await cleanup();
}

Future serverRunning(HttpServer server, {bool useKernel}) async {
Future serverRunning(HttpServer server) async {
int port = server.port;
String inFilePath = "http://127.0.0.1:$port/data/one_line_dart_program.dart";
List<String> args = [inFilePath, "--out=" + outFilePath];
if (!useKernel) args.add(Flags.useOldFrontend);
List<String> args = [
inFilePath,
"--out=" + outFilePath,
Flags.useOldFrontend
];

server.listen(handleRequest);
try {
Expand All @@ -88,32 +93,26 @@ Future serverRunning(HttpServer server, {bool useKernel}) async {
}
}

Future testHttp({bool useKernel}) {
Future testHttp() {
return HttpServer
.bind(InternetAddress.LOOPBACK_IP_V4, 0)
.then((HttpServer server) => serverRunning(server, useKernel: useKernel));
.then((HttpServer server) => serverRunning(server));
}

runTests({bool useKernel}) async {
runTests() async {
tempDir = Directory.systemTemp.createTempSync('directory_test');
outFilePath = path.join(tempDir.path, "out.js");

try {
await testFile(useKernel: useKernel);
if (!useKernel) {
// TODO(johnniwinther): Handle this test for kernel.
await testHttp(useKernel: useKernel);
}
await testFile();
await testHttp();
} finally {
await tempDir.delete(recursive: true);
}
}

main() {
asyncTest(() async {
print('--test from ast---------------------------------------------------');
await runTests(useKernel: false);
print('--test from kernel------------------------------------------------');
await runTests(useKernel: true);
await runTests();
});
}
10 changes: 10 additions & 0 deletions tests/compiler/dart2js_extra/eof_line_ending_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// Regression test derived from language/issue_1578_test.dart with Windows
// line encoding.

main() {}

]~<)$ //# 01: compile-time error

0 comments on commit 969c0de

Please sign in to comment.