Organise imports places @Timeout in the wrong position

[bsutton]

If you are using the test package you are able to add an @timeout annotation to the top of a library to indicate a longer than default timeout for the tests.

The @timeout annotation MUST be at the top of the file.

If you have the @timeout annotation and run 'Organise Imports' the @timeout annotation often gets moved down in amongst the imports and in some cases is completely deleted.

This causes an error an you have to manually move the annotation back to the top of the file or worse still removes it all together and if it is deleted you may not even realise it has been deleted.

Delete Example
In this example 'dart:io' is an unused package. Running organise imports removes dart:io and also @timeout

@Timeout(Duration(minutes: 20))
import 'dart:io';
import 'package:dcli/dcli.dart' hide equals;
import 'package:nginx_le_container/src/util/acquisition_manager.dart';
import 'package:nginx_le_shared/nginx_le_shared.dart';
import 'package:test/test.dart';

After organize imports:

import 'package:dcli/dcli.dart' hide equals;
import 'package:nginx_le_container/src/util/acquisition_manager.dart';
import 'package:nginx_le_shared/nginx_le_shared.dart';
import 'package:test/test.dart';

Move example
The 'dart:io' package is in the wrong place

@Timeout(Duration(minutes: 20))

import 'package:dcli/dcli.dart' hide equals;
import 'package:nginx_le_container/src/util/acquisition_manager.dart';
import 'package:nginx_le_shared/nginx_le_shared.dart';
import 'package:test/test.dart';
import 'dart:io';
import 'mock_cerbot_paths.dart';

In this scenario the @timeout annotation ends up:

import 'dart:io';

@Timeout(Duration(minutes: 20))
import 'package:dcli/dcli.dart' hide equals;
import 'package:nginx_le_container/src/util/acquisition_manager.dart';
import 'package:nginx_le_shared/nginx_le_shared.dart';
import 'package:test/test.dart';

import 'mock_cerbot_paths.dart';

Not exactly a critical bug, but annoying.

[DanTup]

@bwilkerson the Timeout class is decorated with @Target(TargetKind.library) which is documented as:

  /// Indicates that an annotation is valid on the first directive in a library,
  /// whether that's a `library`, `import`, `export` or `part` directive. This
  /// doesn't include the `part of` directive in a part file.
  library,

Would this be the best way to determine whether the annotation should be anchored to the top?

I'm assuming it's not possible to have a single unused import (that would leave nowhere for the annotation to go), as you'd need to always either by importing the file containing the annotation, or if the annotation is in the current file, importing the meta package to be able to decorate it with Target?

[bwilkerson]

Would this be the best way to determine whether the annotation should be anchored to the top?

I can't think of any better way.

I'm assuming it's not possible to have a single unused import ...

Not a single unused import that's annotated with something from meta. You could certainly have an unused import that's annotated with an annotation that's defined locally:

@local
import 'dart:math';

const local = '';

In that case the annotation won't have a Target associated with it. Not sure whether it's better to remove the annotation along with the last directive or whether it's better to leave it attached to the first declaration (assuming there is one). I doubt it will come up often, though.

[DanTup]

This should be fixed with dart-lang/sdk@445ac5d. It's a change in the SDK rather than Dart-Code so will show up with an SDK release.