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

Commit

Permalink
Ensure constructors are sorted first (#186).
Browse files Browse the repository at this point in the history
BUG=
R=scheglov@google.com

Review URL: https://codereview.chromium.org//1706933002 .
  • Loading branch information
pq committed Feb 17, 2016
1 parent 303d246 commit a61ceed
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/src/rules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import 'package:linter/src/rules/package_prefixed_library_names.dart';
import 'package:linter/src/rules/prefer_is_not_empty.dart';
import 'package:linter/src/rules/pub/package_names.dart';
import 'package:linter/src/rules/slash_for_doc_comments.dart';
import 'package:linter/src/rules/sort_constructors_first.dart';
import 'package:linter/src/rules/super_goes_last.dart';
import 'package:linter/src/rules/type_annotate_public_apis.dart';
import 'package:linter/src/rules/type_init_formals.dart';
Expand Down Expand Up @@ -57,6 +58,7 @@ final Registry ruleRegistry = new Registry()
..register(new PackagePrefixedLibraryNames())
..register(new PubPackageNames())
..register(new SlashForDocComments())
..register(new SortConstructorsFirst())
..register(new SuperGoesLast())
..register(new TypeInitFormals())
..register(new TypeAnnotatePublicApis())
Expand Down
67 changes: 67 additions & 0 deletions lib/src/rules/sort_constructors_first.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright (c) 2016, 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.

library linter.src.rules.sort_constructors_first;

import 'package:analyzer/src/generated/ast.dart';
import 'package:linter/src/linter.dart';

const desc = r'Sort constructor declarations before method declarations.';

const details = r'''
**DO** sort constructor declarations before method declarations.
**GOOD:**
```
abstract class Animation<T> {
const Animation();
void addListener(VoidCallback listener);
}
```
**BAD:**
```
abstract class Visitor {
visitSomething(Something s);
Visitor();
}
```
''';

class SortConstructorsFirst extends LintRule {
SortConstructorsFirst()
: super(
name: 'sort_constructors_first',
description: desc,
details: details,
group: Group.style);

@override
AstVisitor getVisitor() => new Visitor(this);
}

class Visitor extends SimpleAstVisitor {
final LintRule rule;
Visitor(this.rule);

@override
visitClassDeclaration(ClassDeclaration decl) {
// Sort members by offset.
List<ClassMember> members = decl.members.toList();
members.sort((ClassMember m1, ClassMember m2) => m1.offset - m2.offset);

bool seenMethod = false;
for (ClassMember member in members) {
if (member is ConstructorDeclaration) {
if (seenMethod) {
rule.reportLint(member.returnType);
}
}
if (member is MethodDeclaration) {

This comment has been minimized.

Copy link
@Hixie

Hixie Mar 2, 2016

It's actually properties and fields that are our biggest offenders.

This comment has been minimized.

Copy link
@pq

pq Mar 3, 2016

Author Member

Interesting. You would like to see constructors before properties?

This comment has been minimized.

Copy link
@Hixie

Hixie Mar 3, 2016

The actual rule is "put the class in order of execution". The constructors run first, so...

We put the fields before the methods that use them. For things that are set by the constructor we normally put them just below the constructors.

seenMethod = true;
}
}
}
}
21 changes: 21 additions & 0 deletions test/rules/sort_constructors_first.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) 2016, 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.

// test w/ `dart test/util/solo_test.dart sort_constructors_first`

abstract class A {
const A();
void f();
}

abstract class B {
void f();
const B(); //LINT
}

abstract class C {
void f();
C(); //LINT
C.named(); //LINT
}

0 comments on commit a61ceed

Please sign in to comment.