dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

Code trap without warnings #2890

Open JCKodel opened 1 year ago

JCKodel commented 1 year ago

Sample code:

import 'package:meta/meta.dart';

abstract class Interface {
  @mustCallSuper
  void preInitialize();

  @mustCallSuper
  Future<void> initialize();
}

abstract class BaseClass implements Interface {}

abstract class UserCodeClass extends BaseClass {
  @override
  Future<void> preInitialize() async {
    print("A");
  }

  @override
  Future<void> initialize() async {
    print("C");
  }
}

class FinalUserCodeClass extends UserCodeClass {
  @override
  Future<void> preInitialize() async {
    await super.preInitialize();
    print("B");
  }

  @override
  Future<void> initialize() async {
    await super.initialize();
    print("D");
  }
}

Future<void> main() async {
  final Interface foo = FinalUserCodeClass();

  print("1");
  foo.preInitialize();

  print("2");
  await foo.initialize();

  print("3");
}

I was building a Flutter package and I made a mistake: my package required a void Function() but I provided a Future<void> Function() in a subclass.

So, while running my code, the order of execution didn't make any sense:

1
A
2
C
B
D
3

B was run after C (I know why: it is because I made a mistake by changing the preInitialize method to be async (even though it isn't in the interface/base class). Code analysis should warn me somehow)

So, the code analysis never said anything about this (changing the overridden signature from void to Future<void>), leading to a mistake where some function required some result from an inheritance that was called later, making a super call be out-of-sync with my expectations.

My analysis_options.yaml:

include: package:flutter_lints/flutter.yaml

analyzer:
  plugins:
    - custom_lint
  language:
    strict-casts: true
    strict-raw-types: true
    strict-inference: true

linter:
  rules:
    always_declare_return_types: true
    always_put_control_body_on_new_line: true
    always_put_required_named_parameters_first: false
    always_require_non_null_named_parameters: true
    annotate_overrides: true
    avoid_empty_else: true
    avoid_field_initializers_in_const_classes: true
    avoid_function_literals_in_foreach_calls: true
    avoid_init_to_null: true
    avoid_null_checks_in_equality_operators: true
    avoid_relative_lib_imports: true
    avoid_renaming_method_parameters: true
    avoid_return_types_on_setters: true
    avoid_returning_null_for_void: true
    avoid_slow_async_io: true
    avoid_types_as_parameter_names: true
    avoid_unused_constructor_parameters: true
    avoid_void_async: true
    await_only_futures: true
    camel_case_types: true
    cancel_subscriptions: true
    control_flow_in_finally: true
    empty_catches: true
    empty_constructor_bodies: true
    empty_statements: true
    flutter_style_todos: false
    hash_and_equals: true
    implementation_imports: true
    iterable_contains_unrelated_type: true
    library_names: true
    library_prefixes: false
    list_remove_unrelated_type: true
    no_adjacent_strings_in_list: true
    no_duplicate_case_values: true
    non_constant_identifier_names: true
    overridden_fields: true
    package_api_docs: true
    package_names: true
    package_prefixed_library_names: true
    prefer_adjacent_string_concatenation: true
    prefer_asserts_in_initializer_lists: true
    prefer_collection_literals: true
    prefer_conditional_assignment: true
    prefer_const_constructors: true
    prefer_const_constructors_in_immutables: true
    prefer_const_declarations: true
    prefer_const_literals_to_create_immutables: true
    prefer_contains: true
    prefer_final_fields: true
    prefer_final_locals: true
    prefer_generic_function_type_aliases: true
    prefer_initializing_formals: true
    prefer_is_empty: true
    prefer_is_not_empty: true
    prefer_iterable_whereType: true
    prefer_typing_uninitialized_variables: true
    prefer_void_to_null: true
    recursive_getters: true
    sort_constructors_first: true
    sort_pub_dependencies: true
    sort_unnamed_constructors_first: true
    test_types_in_equals: true
    throw_in_finally: true
    unawaited_futures: true
    unnecessary_await_in_return: true
    unnecessary_brace_in_string_interps: false
    unnecessary_const: true
    unnecessary_getters_setters: true
    unnecessary_new: true
    unnecessary_null_aware_assignments: true
    unnecessary_null_in_if_null_operators: true
    unnecessary_overrides: true
    unnecessary_parenthesis: true
    unnecessary_statements: true
    unnecessary_this: true
    unrelated_type_equality_checks: true
    use_build_context_synchronously: false
    use_rethrow_when_possible: true
    valid_regexps: true
mateusfccp commented 1 year ago

This happens because void is a top type, and when overriding a method you can override it with any type that is a subtype of the base method.

For a more detailed discussion, see this commentary on a similar issue.

Also, there's already a lint proposal for this case: https://github.com/dart-lang/sdk/issues/58081