Skip to content

Commit 6d1aa6f

Browse files
authored
Refactor Locatable into an interface, HasLocation (#4118)
Since the `mixin` was introduced, and the class modifiers, we've had some tech debt around our mixins. We have a lot of mixins, and some of them are better suited as interfaces or base classes. In this case, `Locatable` was a mixin with mostly abstract getters, and one implemented getter. To me this really smelled like it should be an interface. Turns out, that implemented getter, `documentationIsLocal`, is only called on `Warnable` objects (which implemented `Locatable`). So we can move the getter to `Warnable`. Then we can change every case of `with Locatable` to `implements Locatable`. _Except_ that the interface isn't even needed on some classes, like `Category` and `Package`, so we can just remove the mixin there. Then a rename. I think it's idiomatic for mixin names to end in "able" like "Locatable," "Warnable," "Namable" to suggest the ability that is mixed in. And it is more idiomatic for an interface to start with "Has" to suggest features that are implemented by an implementing class. But there's nothing hard-and-fast here; I'm open to other naming ideas.
1 parent f82cd35 commit 6d1aa6f

File tree

10 files changed

+21
-29
lines changed

10 files changed

+21
-29
lines changed

lib/src/model/accessor.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import 'package:dartdoc/src/warnings.dart';
1616

1717
/// Getters and setters.
1818
class Accessor extends ModelElement {
19-
2019
@override
2120
final PropertyAccessorElement element;
2221

@@ -105,7 +104,7 @@ class Accessor extends ModelElement {
105104
void warn(
106105
PackageWarning kind, {
107106
String? message,
108-
Iterable<Locatable> referredFrom = const [],
107+
Iterable<HasLocation> referredFrom = const [],
109108
Iterable<String> extendedDebug = const [],
110109
}) {
111110
enclosingCombo.warn(kind,

lib/src/model/canonicalization.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ final class _Canonicalization {
203203
}
204204
}
205205

206-
/// A pattern that can split [Locatable.location] strings.
206+
/// A pattern that can split [HasLocation.location] strings.
207207
final _locationSplitter = RegExp(r'(package:|[\\/;.])');
208208

209209
/// This class represents the score for a particular element; how likely

lib/src/model/category.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ class Category
1616
Nameable,
1717
Warnable,
1818
CommentReferable,
19-
Locatable,
2019
MarkdownFileDocumentation,
2120
LibraryContainer,
2221
TopLevelContainer
@@ -96,7 +95,7 @@ class Category
9695
PackageGraph get packageGraph => package.packageGraph;
9796

9897
@override
99-
List<Locatable> get documentationFrom => [this];
98+
List<HasLocation> get documentationFrom => [this];
10099

101100
@override
102101
DocumentLocation get documentedWhere => package.documentedWhere;

lib/src/model/documentation_comment.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ final _htmlInjectRegExp = RegExp(r'<dartdoc-html>([a-f0-9]+)</dartdoc-html>');
3939
/// [_processCommentWithoutTools] and [processComment] are the primary
4040
/// entrypoints.
4141
mixin DocumentationComment
42-
implements Documentable, Warnable, Locatable, SourceCode {
42+
implements Documentable, Warnable, HasLocation, SourceCode {
4343
@override
4444
Element get element;
4545

lib/src/model/locatable.dart

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,15 @@
55
import 'package:dartdoc/src/model/model.dart';
66

77
/// Something that can be located for warning purposes.
8-
mixin Locatable {
8+
abstract interface class HasLocation {
99
/// The [Locatable](s) from which we will get documentation.
1010
///
1111
/// Can be more than one if this is a [Field] composing documentation from
1212
/// multiple [Accessor]s.
1313
///
1414
/// This will walk up the inheritance hierarchy to find docs, if the current
1515
/// class doesn't have docs for this element.
16-
List<Locatable> get documentationFrom;
17-
18-
/// Whether [documentationFrom] contains only one item, `this`.
19-
bool get documentationIsLocal =>
20-
documentationFrom.length == 1 && identical(documentationFrom.first, this);
16+
List<HasLocation> get documentationFrom;
2117

2218
String get fullyQualifiedName;
2319

@@ -36,7 +32,7 @@ mixin Locatable {
3632
bool get isCanonical;
3733
}
3834

39-
extension NullableLocatable on Locatable? {
35+
extension NullableHasLocation on HasLocation? {
4036
String get safeWarnableName =>
4137
this?.fullyQualifiedName.replaceFirst(':', '-') ?? '<unknown>';
4238
}

lib/src/model/model_element.dart

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,8 @@ import 'package:path/path.dart' as p show Context;
5757
/// ModelElement will reference itself as part of the "wrong" [Library] from the
5858
/// public interface perspective.
5959
abstract class ModelElement
60-
with
61-
CommentReferable,
62-
Warnable,
63-
Locatable,
64-
Nameable,
65-
SourceCode,
66-
DocumentationComment
67-
implements Comparable<ModelElement>, Documentable {
60+
with CommentReferable, Warnable, Nameable, SourceCode, DocumentationComment
61+
implements Comparable<ModelElement>, Documentable, HasLocation {
6862
// TODO(jcollins-g): This really wants a "member that has a type" class.
6963
final Element? _originalMember;
7064
final Library _library;

lib/src/model/package.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const String htmlBasePlaceholder = r'%%__HTMLBASE_dartdoc_internal__%%';
3030
/// A [LibraryContainer] that contains [Library] objects related to a particular
3131
/// package.
3232
class Package extends LibraryContainer
33-
with Nameable, Locatable, Warnable, CommentReferable {
33+
with Nameable, Warnable, CommentReferable {
3434
@override
3535
final String name;
3636

@@ -101,7 +101,7 @@ class Package extends LibraryContainer
101101
String? get belowSidebarPath => null;
102102

103103
@override
104-
List<Locatable> get documentationFrom => [this];
104+
List<HasLocation> get documentationFrom => [this];
105105

106106
/// Return true if the code has defined non-default categories for libraries
107107
/// in this package.

lib/src/model/package_graph.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ class PackageGraph with CommentReferable, Nameable {
388388

389389
void warnOnElement(Warnable? warnable, PackageWarning kind,
390390
{String? message,
391-
Iterable<Locatable> referredFrom = const [],
391+
Iterable<HasLocation> referredFrom = const [],
392392
Iterable<String> extendedDebug = const []}) {
393393
var newEntry = (warnable?.element, kind, message);
394394
if (_warnAlreadySeen.contains(newEntry)) {
@@ -406,7 +406,7 @@ class PackageGraph with CommentReferable, Nameable {
406406

407407
void _warnOnElement(Warnable? warnable, PackageWarning kind,
408408
{required String message,
409-
Iterable<Locatable> referredFrom = const [],
409+
Iterable<HasLocation> referredFrom = const [],
410410
Iterable<String> extendedDebug = const []}) {
411411
if (warnable is ModelElement && kind == PackageWarning.ambiguousReexport) {
412412
// This sort of warning is only applicable to top level elements.

lib/src/validator.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ class Validator {
236236
String origin, {
237237
String? referredFrom,
238238
}) {
239-
final referredFromElements = <Locatable>{};
239+
final referredFromElements = <HasLocation>{};
240240

241241
// Make all paths relative to origin.
242242
if (path.isWithin(origin, warnOn)) {

lib/src/warnings.dart

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,20 +94,24 @@ List<DartdocOption<Object?>> createPackageWarningOptions(
9494

9595
/// Something that package warnings can be reported on. Optionally associated
9696
/// with an analyzer [element].
97-
mixin Warnable implements CommentReferable, Documentable, Locatable {
97+
mixin Warnable implements CommentReferable, Documentable, HasLocation {
9898
Element? get element;
9999

100100
void warn(
101101
PackageWarning kind, {
102102
String? message,
103-
Iterable<Locatable> referredFrom = const [],
103+
Iterable<HasLocation> referredFrom = const [],
104104
Iterable<String> extendedDebug = const [],
105105
}) {
106106
packageGraph.warnOnElement(this, kind,
107107
message: message,
108108
referredFrom: referredFrom,
109109
extendedDebug: extendedDebug);
110110
}
111+
112+
/// Whether [documentationFrom] contains only one item, `this`.
113+
bool get documentationIsLocal =>
114+
documentationFrom.length == 1 && identical(documentationFrom.first, this);
111115
}
112116

113117
/// The kinds of warnings that can be displayed when documenting a package.
@@ -326,7 +330,7 @@ enum PackageWarning implements Comparable<PackageWarning> {
326330
String messageForWarnable(Warnable warnable) =>
327331
'$_warnablePrefix ${warnable.safeWarnableName}: ${warnable.location}';
328332

329-
String messageForReferral(Locatable referral) =>
333+
String messageForReferral(HasLocation referral) =>
330334
'$_referredFromPrefix ${referral.safeWarnableName}: ${referral.location}';
331335
}
332336

0 commit comments

Comments
 (0)