Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

enhanced enum support for sort_unnamed_constructors_first #3248

Merged
merged 2 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions lib/src/rules/sort_unnamed_constructors_first.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class SortUnnamedConstructorsFirst extends LintRule {
NodeLintRegistry registry, LinterContext context) {
var visitor = _Visitor(this);
registry.addClassDeclaration(this, visitor);
registry.addEnumDeclaration(this, visitor);
}
}

Expand All @@ -54,10 +55,9 @@ class _Visitor extends SimpleAstVisitor<void> {

_Visitor(this.rule);

@override
void visitClassDeclaration(ClassDeclaration node) {
void check(NodeList<ClassMember> classMembers) {
// Sort members by offset.
var members = node.members.toList()
var members = classMembers.toList()
..sort((ClassMember m1, ClassMember m2) => m1.offset - m2.offset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think of this on the last PR, but I'm kind of surprised that we need to sort the members. I thought we already returned them in source order. Not critical, but it would improve performance to not make a pass confirming that everything is already sorted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! I've made a note to loop back and check on this. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it may be interesting to use a SplayTreeSet instead to keep always the order within the data structure?


var seenConstructor = false;
Expand All @@ -73,4 +73,14 @@ class _Visitor extends SimpleAstVisitor<void> {
}
}
}

@override
void visitClassDeclaration(ClassDeclaration node) {
check(node.members);
}

@override
void visitEnumDeclaration(EnumDeclaration node) {
check(node.members);
}
}
3 changes: 3 additions & 0 deletions test/rules/all.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ import 'prefer_generic_function_type_aliases_test.dart'
import 'prefer_spread_collections_test.dart' as prefer_spread_collections;
import 'public_member_api_docs_test.dart' as public_member_api_docs;
import 'sort_constructors_first.dart' as sort_constructors_first;
import 'sort_unnamed_constructors_first.dart'
as sort_unnamed_constructors_first;
import 'super_goes_last_test.dart' as super_goes_last;
import 'tighten_type_of_initializing_formals_test.dart'
as tighten_type_of_initializing_formals;
Expand Down Expand Up @@ -106,6 +108,7 @@ void main() {
prefer_spread_collections.main();
public_member_api_docs.main();
sort_constructors_first.main();
sort_unnamed_constructors_first.main();
super_goes_last.main();
tighten_type_of_initializing_formals.main();
type_init_formals.main();
Expand Down
46 changes: 46 additions & 0 deletions test/rules/sort_unnamed_constructors_first.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) 2022, 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.

import 'package:test_reflective_loader/test_reflective_loader.dart';

import '../rule_test_support.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(SortUnnamedConstructorsFirstTest);
});
}

@reflectiveTest
class SortUnnamedConstructorsFirstTest extends LintRuleTest {
@override
List<String> get experiments => [
EnableString.enhanced_enums,
];

@override
String get lintRule => 'sort_unnamed_constructors_first';

test_ok() async {
await assertNoDiagnostics(r'''
enum A {
a,b,c.aa();
const A();
const A.aa();
}
''');
}

test_unsorted() async {
await assertDiagnostics(r'''
enum A {
a,b,c.aa();
const A.aa();
const A();
}
''', [
lint('sort_unnamed_constructors_first', 47, 1),
]);
}
}