dart-lang / co19

A Dart language and library conformance test suite
BSD 3-Clause "New" or "Revised" License
37 stars 28 forks source link

co19/LanguageFeatures/nnbd/legacy_libraries_A04_t05 #549

Closed scheglov closed 4 years ago

scheglov commented 4 years ago

It is definitely an error, but maybe not so nice error recovery.

Analyzer reports:

error: Expected to find '}'. (expected_token at [test] bin/test.dart:17)
error: Undefined class 'required'. (undefined_class at [test] bin/test.dart:17)
error: Undefined name 's'. (undefined_identifier at [test] bin/test.dart:22)
error: The named parameter 'named' isn't defined. (undefined_named_parameter at [test] bin/test.dart:22)

I'm not sure if we should specify which error is reported. Also, FWIW, s is not declared in the test.

--- Command "dart2analyzer" (took 89ms):
DART_CONFIGURATION=ReleaseX64NNBD out/ReleaseX64NNBD/dart-sdk/bin/dartanalyzer --enable-experiment=non-nullable --ignore-unrecognized-flags --packages=/b/s/w/ir/cache/builder/sdk/.packages --format=machine --no-hints /b/s/w/ir/cache/builder/sdk/tests/co19/src/LanguageFeatures/nnbd/legacy_libraries_A04_t05.dart

static error failures:
Unexpected static error at line 22, column 7, length 5:
- Had error code COMPILE_TIME_ERROR.UNDEFINED_NAMED_PARAMETER.

Unexpected static error at line 22, column 14, length 1:
- Had error code STATIC_WARNING.UNDEFINED_IDENTIFIER.

--- Re-run this test:
python tools/test.py -n analyzer-asserts-strong-linux co19/LanguageFeatures/nnbd/legacy_libraries_A04_t05
scheglov commented 4 years ago

I still see mismatches between expected and reported errors.

--- Command "dart2analyzer" (took 21ms):
DART_CONFIGURATION=ReleaseX64NNBD out/ReleaseX64NNBD/dart-sdk/bin/dartanalyzer --enable-experiment=non-nullable --ignore-unrecognized-flags --packages=/b/s/w/ir/cache/builder/sdk/.packages --format=machine --no-hints /b/s/w/ir/cache/builder/sdk/tests/co19/src/LanguageFeatures/nnbd/legacy_libraries_A04_t05.dart

static error failures:
Unexpected static error at line 17, column 6, length 8:
- Had error code COMPILE_TIME_ERROR.UNDEFINED_CLASS.

Unexpected static error at line 17, column 22, length 5:
- Had error code SYNTACTIC_ERROR.EXPECTED_TOKEN.

--- Re-run this test:
python tools/test.py -n analyzer-asserts-strong-linux co19/LanguageFeatures/nnbd/legacy_libraries_A04_t05
scheglov commented 4 years ago

Is there anything to fix here?

iarkh commented 4 years ago

Seems like all is OK now. Which version of dart you tested?

scheglov commented 4 years ago

I use the bleeding edge version (build it from the master branch). It reports expectations mismatch as described in the earlier comment.

I wonder if something should be fixed in the test (expect just an error, or update the test to expect errors that are reported, even if they are not ideal). But maybe the test was updated, and just not. yet rolled into the SDK. I have the following file:

/*
 * Copyright (c) 2019, 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.
 */
/**
 * @assertion  In a legacy library, none of the new syntax introduced by this
 * proposal is available, and it is a static error if it is used.
 *
 * @description Check that it is a static error if NNBD syntax is used in a
 * pre-NNBD legacy library
 * @author sgrekhov@unipro.ru
 */
// @dart=2.6
// SharedOptions=--enable-experiment=non-nullable

foo({required String named}) {}
//   ^^^^^^^^        ^^^^^
// [analyzer] unspecified
// [cfe] unspecified
main() {
  foo(named: "Lily was here");
//    ^^^^^
// [analyzer] unspecified
// [cfe] unspecified
}
sgrekhov commented 4 years ago

@scheglov you are using the latest version of the test. And there ARE expectation of the errors on line 17 on the right places. 'unspecified' means any error. So the test should pass. I believe, if test fails, it is an issue of the test runner in case when there are two expected errors on the same line. @athomas is it allowed to expect two or more errors on the same line?

eernstg commented 4 years ago

There is support for more than one error on the same line, cf. language_2/argument/not_enough_positional_arguments_test.dart.

sgrekhov commented 4 years ago

@eernstg thank youfor pointing! It's strange, but it doesn't work for me. I tried to change the test in the following way

/*
 * Copyright (c) 2019, 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.
 */
/**
 * @assertion  In a legacy library, none of the new syntax introduced by this
 * proposal is available, and it is a static error if it is used.
 *
 * @description Check that it is a static error if NNBD syntax is used in a
 * pre-NNBD legacy library
 * @author sgrekhov@unipro.ru
 */
// @dart=2.6
// SharedOptions=--enable-experiment=non-nullable

foo({required String named}) {}
//   ^^^^^^^^
// [analyzer] unspecified
// [cfe] unspecified
//                   ^^^^^                <- this is line 21
// [analyser] unspecified
// [cfe] unspecified

main() {
  foo(named: "Lily was here");
//    ^^^^^
// [analyzer] unspecified
// [cfe] unspecified
}

When I'm trying to run the test I get the following error

sergey@sgrekhov-ubuntu:~/dart-sdk/sdk$ tools/test.py -n analyzer-asserts-strong-linux co19/LanguageFeatures/nnbd/legacy_libraries_A04_t05
Test configuration:
    analyzer-asserts-strong-linux(architecture: x64, compiler: dart2analyzer, mode: release, runtime: none, system: linux, nnbd: strong, enable-experiment: [non-nullable], enable-asserts, use-sdk)
Suites tested: co19
Unhandled exception:
FormatException: Invalid error expectation syntax in /home/sergey/dart-sdk/sdk/tests/co19/src/LanguageFeatures/nnbd/legacy_libraries_A04_t05.dart:
FormatException: Test error on line 21: An error expectation must specify at least an analyzer or CFE error.

Why it doesn't work?

UPD. There was [analySer] instead of [analyZer]

scheglov commented 4 years ago

Thank you for updating the test. The set of expectations looked reasonable to me, but it did not work.

I looked into test_runner that matches expectations and actual errors, and see when the error is unspecified, you should not provide it more than once on the same line. So, the test should be:

foo({required String named}) {}
//   ^^^^^^^^
// [analyzer] unspecified
// [cfe] unspecified
main() {
  foo(named: "Lily was here");
//    ^^^^^
// [analyzer] unspecified
// [cfe] unspecified
}
scheglov commented 4 years ago

Another solution would be to split the line into two, and verify each line with separate undefined expectation.

sgrekhov commented 4 years ago

Thank you. I splitted the line with two expected error to two ones