Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Style - in what order do imports, parts, and other preamble stuff happen? #635

Closed
nicolasgarnier opened this issue Aug 26, 2014 · 23 comments

Comments

@nicolasgarnier
Copy link
Contributor

From [email protected] on April 09, 2013 18:56:24

The style guide should say in what order the stuff at the top of the page goes.

I think it looks like:

License (if any)
<2 blank lines>
Library doc comment
library name; (prefer library name)

part of foo;
<2 blank lines>
"dart:" imports
<1 blank line>
"package:" imports
<1 blank line>
"file:" imports
<1 blank line>
exports alphabetized by URI - show should come before hide within the same export
<1 blank line>
parts
<2 blank lines>
rest of the file

all imports , parts, etc. should be alphabetized.

Original issue: http://code.google.com/p/dart/issues/detail?id=9806

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on July 24, 2013 14:02:12

Labels: -Type-Documentation -Area-Site Docs-Articles Area-Documentation

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on September 13, 2013 14:29:02

Labels: Docs-StyleGuide

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on September 13, 2013 14:29:53

Labels: -Docs-Articles

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on September 23, 2013 12:12:51

I'm not a fan of two blank lines. For pub, we do:

// License...

/// Library doc comment.
library foo;

import "dart:foo";
import "dart:bar";

import "package:foo";
import "package:bar";

import "foo.dart";
import "bar.dart";

export "foo.dart";
export "bar.dart";

code...

In other words:

  1. License.
  2. Library doc comment.
  3. Library tag.
  4. "dart:" imports.
  5. "package:" imports.
  6. Relative imports.
  7. Exports.
  8. Code.

With a single blank line between all of those except 2 and 3.

I'm OK with the style guide suggesting this, but I'd be OK with not worrying about it either.

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on October 09, 2013 00:33:06

I tend to import stuff in the order I find them most relevant to the current library.

That may put packages that I'm highly dependent on (in tests it's unittest and the library being tested) before dart:-imports that are just imported for a single class or two).

Less important libraries will usually get a "show" part as well, to just give the one or two classes I need, where the main dependencies are imported in full.

That obviously isn't a clear guideline, but it makes more sense when reading the imports than just having a forced but irrelevant order (which just makes the imports a meaningless blob best hidden by a folding editor, like it typically is in Java).

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on November 18, 2013 12:50:55

That may put packages that I'm highly dependent on (in tests it's unittest and the library being tested) before dart:-imports that are just imported for a single class or two).

I've tended to do that before, but honestly I find that too fuzzy to define even in my own code.

That obviously isn't a clear guideline, but it makes more sense when reading the imports than just having a forced but irrelevant order

The order probably doesn't matter much to someone reading the imports anyway. I think they just want to know what is imported. Having a mechanical rule makes it possible to automate the formatting here.

Do I get to make unilateral style guide decisions? If so: let it be known that the decision here is to use the order in comment #4.

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on December 06, 2013 00:13:07

The only real question here is the order of imports/exports (and whether you should have one or two empty lines between stuff). The order of the remaining stuff is pretty much fixed by the language.

I still vote against forcing an order on imports. We can recommend something (e.g., "if you have no reason to order imports differently, we suggest you order them alphabetically").
I also tend to put exports next to imports of the same library, e.g.:
import "package:foo/bar.dart";
export "package:foo/bar.dart" show TheFoo;
That's a logical grouping that makes it easier to understand what is going on.

(So, obviously, I don't think Bob should unilaterally pick something that I don't want :)

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on December 06, 2013 09:07:59

This falls into the bucket of guidelines where I think:

  1. We should have a well-defined ordering so that tooling can automate it for users who want to invoke that.
  2. Users who hand-author this should not be mandated to follow the ordering.

In other words, if you don't care, we should care on your behalf. If you do care, we'll get out of your way.

@nicolasgarnier
Copy link
Contributor Author

From butler.matthew on December 06, 2013 09:16:16

FWIW, I generally tend to follow the same pattern as mentioned in Comment #4 The additional comment being in part 6 (Relative imports) will also be package imports for my own current package.

eg: file in web/myApp.dart

library

import 'dart:html';
import 'dart:convert' show JSON;

import 'package:foo/foo.dart'

import 'relative.dart';
import 'package:my_app/my_app.dart';

// code

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on December 06, 2013 11:54:52

What #8 says.

The style guide does have "PREFER" for things that are what we think are good, but that are not obligatory.

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on December 10, 2013 00:29:34

Issue 15551 has been merged into this issue.

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on January 29, 2014 07:38:20

Labels: Type-Enhancement

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on May 22, 2014 15:45:12

Cc: [email protected]

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on June 04, 2014 14:58:11

Labels: -Area-Documentation Area-Site

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on July 10, 2014 03:20:32

Labels: -Milestone-Later Oldschool-Milestone-Later

@nicolasgarnier
Copy link
Contributor Author

From [email protected] on August 04, 2014 00:43:01

Labels: -Oldschool-Milestone-Later

@munificent
Copy link
Contributor

I need to do a little research here to see what code in the wild is doing.

@pq
Copy link
Contributor

pq commented Feb 13, 2015

👍 for getting this settled. "Organize imports" is a much valued action in most IDEs. More FWIW, analysis_server currently takes a stab at this. It's surfaced in DartEditor as the "Sort unit/class members" action. I think it follows Bob's suggestion above but will defer to @scheglov for the details.

@seaneagan
Copy link

Here's a try at the actual rules:

  • DO specify dart: imports before package: imports before relative imports, with each section alphabetically sorted and separated from the previous section by a single empty line.
  • PREFER to specify exports in a separate section after all imports and a single empty line.
  • PREFER to specify a library name for all libraries.

Thoughts?

@munificent
Copy link
Contributor

That sounds good to me.

@kwalrath
Copy link
Contributor

I'll prep a CL.

@kwalrath kwalrath assigned kwalrath and unassigned munificent Feb 20, 2015
@kwalrath kwalrath added started and removed triaged labels Feb 20, 2015
@kwalrath
Copy link
Contributor

https://chromiumcodereview.appspot.com/959553002 covers import & export order, as well as specifying a library. It doesn't cover the license or the order of license, library, and code.

@kwalrath
Copy link
Contributor

License probably must go at the top, for legal reasons. Code must go after imports, for language reasons. We don't need to specify those.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

6 participants