Skip to content

Fix lints and warnings accumulated in dartdoc #1635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 16, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
28 changes: 11 additions & 17 deletions bin/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ main(List<String> arguments) async {
output['message'] = record.message;
}

print(JSON.encode(output));
print(json.encode(output));
});
} else {
final stopwatch = new Stopwatch()..start();
Expand Down Expand Up @@ -311,29 +311,24 @@ ArgParser _createArgsParser() {
help: 'Path to source directory.', defaultsTo: Directory.current.path);
parser.addOption('output',
help: 'Path to output directory.', defaultsTo: defaultOutDir);
parser.addOption('header',
allowMultiple: true,
parser.addMultiOption('header',
splitCommas: true,
help: 'paths to header files containing HTML text.');
parser.addOption('footer',
allowMultiple: true,
parser.addMultiOption('footer',
splitCommas: true,
help: 'paths to footer files containing HTML text.');
parser.addOption('footer-text',
allowMultiple: true,
parser.addMultiOption('footer-text',
splitCommas: true,
help: 'paths to footer-text files '
'(optional text next to the package name and version).');
parser.addOption('exclude',
allowMultiple: true, splitCommas: true, help: 'Library names to ignore.');
parser.addOption('exclude-packages',
allowMultiple: true, splitCommas: true, help: 'Package names to ignore.');
parser.addOption('include',
allowMultiple: true,
parser.addMultiOption('exclude',
splitCommas: true, help: 'Library names to ignore.');
parser.addMultiOption('exclude-packages',
splitCommas: true, help: 'Package names to ignore.');
parser.addMultiOption('include',
splitCommas: true,
help: 'Library names to generate docs for.');
parser.addOption('include-external',
allowMultiple: true,
parser.addMultiOption('include-external',
help: 'Additional (external) dart files to include; use "dir/fileName", '
'as in lib/material.dart.');
parser.addOption('hosted-url',
Expand All @@ -353,10 +348,9 @@ ArgParser _createArgsParser() {
help: 'Group libraries from the same package into categories.',
negatable: false,
defaultsTo: false);
parser.addOption('category-order',
parser.addMultiOption('category-order',
help: 'A list of category names to place first when --use-categories is '
'set. Unmentioned categories are sorted after these.',
allowMultiple: true,
splitCommas: true);
parser.addFlag('auto-include-dependencies',
help:
Expand Down
4 changes: 2 additions & 2 deletions lib/src/html/resource_loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
library dartdoc.resource_loader;

import 'dart:async' show Future;
import 'dart:convert' show UTF8;
import 'dart:convert' show utf8;

import 'package:resource/resource.dart';

/// Loads a `package:` resource as a String.
Future<String> loadAsString(String path) async {
var bytes = await loadAsBytes(path);

return UTF8.decode(bytes);
return utf8.decode(bytes);
}

/// Loads a `package:` resource as an [List<int>].
Expand Down
2 changes: 1 addition & 1 deletion lib/src/io_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class SubprocessLauncher {
assert(prefix != null);
if (filter == null) filter = (line) => [line];
stream
.transform(UTF8.decoder)
.transform(utf8.decoder)
.transform(const LineSplitter())
.expand(filter)
.listen((String line) {
Expand Down
12 changes: 6 additions & 6 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,15 @@ final RegExp notARealDocReference = new RegExp(r'''(^[^\w]|^[\d]|[,"'/]|^$)''');

final RegExp operatorPrefix = new RegExp(r'^operator[ ]*');

final HtmlEscape htmlEscape = const HtmlEscape(HtmlEscapeMode.ELEMENT);
final HtmlEscape htmlEscape = const HtmlEscape(HtmlEscapeMode.element);

final List<md.InlineSyntax> _markdown_syntaxes = [
new _InlineCodeSyntax(),
new _AutolinkWithoutScheme()
]..addAll(md.ExtensionSet.gitHub.inlineSyntaxes);
]..addAll(md.ExtensionSet.gitHubFlavored.inlineSyntaxes);

final List<md.BlockSyntax> _markdown_block_syntaxes = []
..addAll(md.ExtensionSet.gitHub.blockSyntaxes);
..addAll(md.ExtensionSet.gitHubFlavored.blockSyntaxes);

// Remove these schemas from the display text for hyperlinks.
final RegExp _hide_schemes = new RegExp('^(http|https)://');
Expand Down Expand Up @@ -735,7 +735,7 @@ String _linkDocReference(
// This would be linkedElement.linkedName, but link bodies are slightly
// different for doc references.
if (linkedElement.href == null) {
return '<code>${HTML_ESCAPE.convert(label)}</code>';
return '<code>${htmlEscape.convert(label)}</code>';
} else {
return '<a ${classContent}href="${linkedElement.href}">$label</a>';
}
Expand All @@ -744,7 +744,7 @@ String _linkDocReference(
warnable.warn(PackageWarning.unresolvedDocReference,
message: codeRef, referredFrom: warnable.documentationFrom);
}
return '<code>${HTML_ESCAPE.convert(label)}</code>';
return '<code>${htmlEscape.convert(label)}</code>';
}
}

Expand Down Expand Up @@ -992,7 +992,7 @@ class _InlineCodeSyntax extends md.InlineSyntax {

@override
bool onMatch(md.InlineParser parser, Match match) {
var element = new md.Element.text('code', HTML_ESCAPE.convert(match[1]));
var element = new md.Element.text('code', htmlEscape.convert(match[1]));
parser.addNode(element);
return true;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Map<String, Map<String, List<Map<String, dynamic>>>> get _crossdartJson {
var crossdartFile =
new File(p.join(config.inputDir.path, "crossdart.json"));
if (crossdartFile.existsSync()) {
__crossdartJson = JSON.decode(crossdartFile.readAsStringSync())
__crossdartJson = json.decode(crossdartFile.readAsStringSync())
as Map<String, Map<String, List<Map<String, dynamic>>>>;
} else {
__crossdartJson = {};
Expand Down Expand Up @@ -1543,7 +1543,7 @@ abstract class GetterSetterCombo implements ModelElement {

String _buildConstantValueBase() {
String result = constantInitializer?.toString() ?? '';
return const HtmlEscape(HtmlEscapeMode.UNKNOWN).convert(result);
return const HtmlEscape(HtmlEscapeMode.unknown).convert(result);
}

String get constantValue => linkifyConstantValue(constantValueBase);
Expand Down Expand Up @@ -3413,7 +3413,7 @@ abstract class ModelElement extends Canonicalization
if (isPublicAndPackageDocumented) {
warn(PackageWarning.noCanonicalFound);
}
return HTML_ESCAPE.convert(name);
return htmlEscape.convert(name);
}

var classContent = isDeprecated ? ' class="deprecated"' : '';
Expand Down
2 changes: 1 addition & 1 deletion pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,4 @@ packages:
source: hosted
version: "2.1.13"
sdks:
dart: ">=2.0.0-dev.23.0 <=2.0.0-dev.36.0"
dart: ">=2.0.0-dev.23.0 <=2.0.0-dev.38.0"
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ homepage: https://github.com/dart-lang/dartdoc
# For development, recommend 1.25.0-dev.0.0 or higher to allow
# dartanalyzer to work on dartdoc itself.
environment:
sdk: '>=1.23.0-dev.11.5 <2.0.0'
sdk: '>=2.0.0-dev.9.0 <3.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

This constraint is handy to know. Thanks!

dependencies:
analyzer: '0.31.2-alpha.0'
args: '>=0.13.0 <2.0.0'
Expand Down
2 changes: 1 addition & 1 deletion test/compare_output_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void main() {

var jsonValues = LineSplitter
.split(result.stdout)
.map((j) => JSON.decode(j) as Map<String, dynamic>)
.map((j) => json.decode(j) as Map<String, dynamic>)
.toList();

expect(jsonValues, isNotEmpty,
Expand Down
10 changes: 0 additions & 10 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1172,20 +1172,15 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
});

test('typedef params have proper signature', () {
// typedef void VoidCallback();
// void addCallback(VoidCallback callback) { }
ModelFunction function =
fakeLibrary.functions.firstWhere((f) => f.name == 'addCallback');
ElementType t = function.parameters.first.modelType;
String params = function.linkedParams();
expect(
params,
'<span class="parameter" id="addCallback-param-callback">'
'<span class="type-annotation"><a href="fake/VoidCallback.html">VoidCallback</a></span> '
'<span class="parameter-name">callback</span></span>');

// typedef int Callback2(String);
// void addCallback2(Callback2 callback) { }
function =
fakeLibrary.functions.firstWhere((f) => f.name == 'addCallback2');
params = function.linkedParams();
Expand Down Expand Up @@ -1216,7 +1211,6 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
.singleWhere((f) => f.name == 'explicitSetter');
// TODO(jcollins-g): really, these shouldn't be called "parameters" in
// the span class.
ElementType t = explicitSetter.modelType;
expect(explicitSetter.linkedReturnType,
'<span class="parameter" id="explicitSetter=-param-f"><span class="type-annotation">dynamic</span> <span class="parameter-name">Function</span>(<span class="parameter" id="f-param-bar"><span class="type-annotation">int</span>, </span> <span class="parameter" id="f-param-baz"><span class="type-annotation"><a href="fake/Cool-class.html">Cool</a></span>, </span> <span class="parameter" id="f-param-macTruck"><span class="type-annotation">List<span class="signature">&lt;int&gt;</span></span></span>)</span>');
});
Expand Down Expand Up @@ -1293,7 +1287,6 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
Method aTypedefReturningMethodInterface = TemplatedInterface
.allInstanceMethods
.singleWhere((m) => m.name == 'aTypedefReturningMethodInterface');
ElementType mt = aTypedefReturningMethodInterface.modelType;
expect(aTypedefReturningMethodInterface.linkedReturnType,
'<a href=\"ex/ParameterizedTypedef.html\">ParameterizedTypedef</a><span class="signature">&lt;List<span class="signature">&lt;String&gt;</span>&gt;</span>');
});
Expand Down Expand Up @@ -2240,7 +2233,6 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
});

test('anonymous nested functions inside typedefs are handled', () {
ElementType t = aComplexTypedef.modelType;
expect(aComplexTypedef, isNotNull);
expect(aComplexTypedef.linkedReturnType, startsWith('Function'));
expect(aComplexTypedef.nameWithGenerics,
Expand Down Expand Up @@ -2371,13 +2363,11 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
});

test('typedef param is linked and does not include types', () {
ElementType t = methodWithTypedefParam.parameters.first.modelType;
var params = methodWithTypedefParam.linkedParams();
expect(
params,
equals(
'<span class="parameter" id="methodWithTypedefParam-param-p"><span class="type-annotation"><a href="ex/processMessage.html">processMessage</a></span> <span class="parameter-name">p</span></span>'));
//expect(params, contains('<a href="ex/processMessage.html">'));
});
});

Expand Down
16 changes: 12 additions & 4 deletions testing/test_package/lib/fake.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,10 @@
library fake;

import 'dart:async';

import 'dart:collection';

import 'example.dart';

import 'css.dart' as css;

import 'example.dart';
import 'two_exports.dart' show BaseClass;

abstract class ImplementingThingy implements BaseThingy {}
Expand Down Expand Up @@ -224,6 +221,7 @@ typedef int LotsAndLotsOfParameters(so, many, parameters, it, should, wrap,

/// This class is cool!
class Cool {
// ignore: missing_return
Cool returnCool() {}
Copy link
Member

Choose a reason for hiding this comment

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

Aside: why not just return null? In general, I'm curious why all the ignores and not just fixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test package is intended to be a bit strange, using outdated, error-prone and/or obsolete constructions in some places. So I don't want to accidentally make the test package too "clean".

I shouldn't be doing that anywhere outside testing/ though.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Thanks!

}

Expand Down Expand Up @@ -258,7 +256,9 @@ class SuperAwesomeClass {
///
/// Another comment line.
void fly(int height, Cool superCool, {String msg}) {
// ignore: unused_local_variable, avoid_init_to_null
var x = null;
// ignore: unused_local_variable
int i, y;
for (int z = 0; z < 100; z++) {
print('hi');
Expand Down Expand Up @@ -400,6 +400,7 @@ class ClassWithUnusualProperties extends ImplicitProperties {
///
/// The rest of this is not in the first paragraph.
@Annotation('value')
// ignore: deprecated_member_use
class LongFirstLine extends SuperAwesomeClass
with MixMeIn
implements Interface, AnotherInterface {
Expand Down Expand Up @@ -649,14 +650,18 @@ class HasGenericWithExtends<T extends Foo2> {}

/// Extends [ListBase]
class SpecialList<E> extends ListBase<E> {
// ignore: annotate_overrides
E operator [](int index) {
return null;
}

// ignore: annotate_overrides
int get length => 0;

// ignore: annotate_overrides
void set length(int length) {}

// ignore: annotate_overrides
void operator []=(int index, E value) {}
}

Expand All @@ -676,6 +681,7 @@ class BaseForDocComments {
///
/// Reference to another method in this class [anotherMethod] xx
///
// ignore: deprecated_member_use
/// Reference to a top-level function in this library [topLevelFunction] xx
///
/// Reference to a top-level function in another library that is imported into this library (example lib) [function1] xx
Expand Down Expand Up @@ -771,6 +777,7 @@ class ReferringClass {
abstract class MIEEMixinWithOverride<K, V> = MIEEBase<K, V> with _MIEEPrivateOverride<K, V>;

abstract class _MIEEPrivateOverride<K, V> implements MIEEThing<K, V> {
// ignore: annotate_overrides
void operator[]=(K key, V value) {
throw new UnsupportedError("Never use this");
}
Expand All @@ -779,6 +786,7 @@ abstract class _MIEEPrivateOverride<K, V> implements MIEEThing<K, V> {
abstract class MIEEBase<K, V> extends MIEEMixin<K, V> {}

abstract class MIEEMixin<K, V> implements MIEEThing<K, V> {
// ignore: annotate_overrides
operator []=(K key, V value);
}

Expand Down
14 changes: 7 additions & 7 deletions tool/doc_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
library dartdoc.doc_packages;

import 'dart:async';
import 'dart:convert' show JSON, UTF8;
import 'dart:convert' show json, utf8;
import 'dart:io';

import 'package:args/args.dart';
Expand Down Expand Up @@ -125,17 +125,17 @@ Future<List<String>> _packageUrls(int page) {
return http
.get('https://pub.dartlang.org/packages.json?page=${page}')
.then((response) {
return new List<String>.from(JSON.decode(response.body)['packages']);
return new List<String>.from(json.decode(response.body)['packages']);
});
}

Future<List<PackageInfo>> _getPackageInfos(List<String> packageUrls) {
var futures = packageUrls.map((String p) {
return http.get(p).then((response) {
var json = JSON.decode(response.body);
String name = json['name'];
var decodedJson = json.decode(response.body);
String name = decodedJson['name'];
List<Version> versions = new List<Version>.from(
json['versions'].map((v) => new Version.parse(v)));
decodedJson['versions'].map((v) => new Version.parse(v)));
return new PackageInfo(name, Version.primary(versions));
});
}).toList();
Expand Down Expand Up @@ -200,8 +200,8 @@ Future _exec(String command, List<String> args,
.start(command, args, workingDirectory: cwd)
.then((Process process) {
if (!quiet) {
process.stdout.listen((bytes) => _log(UTF8.decode(bytes)));
process.stderr.listen((bytes) => _log(UTF8.decode(bytes)));
process.stdout.listen((bytes) => _log(utf8.decode(bytes)));
process.stderr.listen((bytes) => _log(utf8.decode(bytes)));
}

Future f = process.exitCode.then((code) {
Expand Down