Chromium Code Reviews| OLD | NEW |
|---|---|
| (Empty) | |
| 1 // Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file | |
| 2 // for details. All rights reserved. Use of this source code is governed by a | |
| 3 // BSD-style license that can be found in the LICENSE file. | |
| 4 | |
| 5 library linter.src.rules.implementation_imports; | |
| 6 | |
| 7 import 'package:analyzer/src/generated/ast.dart' | |
| 8 show AstVisitor, ImportDirective, SimpleAstVisitor; | |
| 9 import 'package:linter/src/linter.dart'; | |
| 10 | |
| 11 const desc = "Don't import implementation files from another package."; | |
| 12 | |
| 13 const details = ''' | |
| 14 **DON'T** import implementation files from another package. | |
| 15 | |
| 16 From the the [pub package layout doc] | |
| 17 (https://www.dartlang.org/tools/pub/package-layout.html#implementation-files): | |
| 18 | |
| 19 The libraries inside `lib` are publicly visible: other packages are free to | |
| 20 import them. But much of a package's code is internal implementation libraries | |
| 21 that should only be imported and used by the package itself. Those go inside a | |
| 22 subdirectory of `lib` called `src`. You can create subdirectories in there if | |
| 23 it helps you organize things. | |
| 24 | |
| 25 You are free to import libraries that live in `lib/src` from within other Dart | |
| 26 code in the same package (like other libraries in `lib`, scripts in `bin`, | |
| 27 and tests) but you should never import from another package's `lib/src` | |
| 28 directory. Those files are not part of the package’s public API, and they | |
| 29 might change in ways that could break your code. | |
| 30 | |
| 31 **Bad:** | |
| 32 | |
| 33 ``` | |
| 34 // In 'road_runner' | |
| 35 import 'package:acme/lib/src/internals.dart; | |
| 36 ``` | |
| 37 '''; | |
| 38 | |
| 39 bool isImplementation(Uri uri) { | |
| 40 List<String> segments = uri?.pathSegments ?? const []; | |
| 41 if (segments.length > 2) { | |
| 42 if (segments[1] == 'src') { | |
| 43 return true; | |
| 44 } | |
| 45 } | |
| 46 return false; | |
| 47 } | |
| 48 | |
| 49 bool isPackage(Uri uri) => uri?.scheme == 'package' ?? false; | |
|
scheglov
2015/10/31 21:25:12
I think "?? false" is not necessary, because null
pquitslund
2015/10/31 23:13:08
Spot on. I extracted this method and missed that
| |
| 50 | |
| 51 bool samePackage(Uri uri1, Uri uri2) { | |
| 52 var segments1 = uri1.pathSegments; | |
| 53 var segments2 = uri2.pathSegments; | |
| 54 if (segments1.length < 1 || segments2.length < 1) { | |
| 55 return false; | |
| 56 } | |
| 57 return segments1[0] == segments2[0]; | |
| 58 } | |
| 59 | |
| 60 class ImplementationImports extends LintRule { | |
| 61 ImplementationImports() | |
| 62 : super( | |
| 63 name: 'implementation_imports', | |
| 64 description: desc, | |
| 65 details: details, | |
| 66 group: Group.style); | |
| 67 | |
| 68 @override | |
| 69 AstVisitor getVisitor() => new Visitor(this); | |
| 70 } | |
| 71 | |
| 72 class Visitor extends SimpleAstVisitor { | |
| 73 LintRule rule; | |
|
Brian Wilkerson
2015/10/31 21:04:30
Make 'rule' final?
pquitslund
2015/10/31 23:13:09
Done. Aside: I'm puzzled as to why I haven't done
| |
| 74 Visitor(this.rule); | |
| 75 | |
| 76 @override | |
| 77 visitImportDirective(ImportDirective node) { | |
| 78 Uri importUri = node?.source?.uri; | |
| 79 Uri sourceUri = node?.element?.source?.uri; | |
| 80 | |
| 81 // Test for 'package:*/src/'. | |
| 82 if (!isImplementation(importUri)) { | |
| 83 return; | |
| 84 } | |
| 85 | |
| 86 // If the source URI is not a `package` URI bail out. | |
| 87 if (!isPackage(sourceUri)) { | |
| 88 return; | |
| 89 } | |
| 90 | |
| 91 if (!samePackage(importUri, sourceUri)) { | |
| 92 rule.reportLint(node.uri); | |
| 93 } | |
| 94 } | |
| 95 } | |
| OLD | NEW |