Skip to content

Commit

Permalink
Remove all dynamic calls and properly enforce lint rule (#2582)
Browse files Browse the repository at this point in the history
  • Loading branch information
srawlins authored Mar 18, 2021
1 parent cd4fa41 commit 52e50c9
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 58 deletions.
1 change: 1 addition & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ analyzer:
linter:
rules:
- always_declare_return_types
- avoid_dynamic_calls
- avoid_single_cascade_in_expression_statements
- avoid_unused_constructor_parameters
- annotate_overrides
Expand Down
1 change: 1 addition & 0 deletions analysis_options_presubmit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ analyzer:
linter:
rules:
- always_declare_return_types
- avoid_dynamic_calls
- avoid_single_cascade_in_expression_statements
- avoid_unused_constructor_parameters
- annotate_overrides
Expand Down
54 changes: 27 additions & 27 deletions lib/src/dartdoc_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -360,32 +360,32 @@ class ToolConfiguration {
List<String> findCommand([String prefix = '']) {
List<String> command;
// If the command key is given, then it applies to all platforms.
var commandFrom = toolMap.containsKey('${prefix}command')
var commandFromKey = toolMap.containsKey('${prefix}command')
? '${prefix}command'
: '$prefix${Platform.operatingSystem}';
if (toolMap.containsKey(commandFrom)) {
if (toolMap[commandFrom].value is String) {
command = [toolMap[commandFrom].toString()];
if (toolMap.containsKey(commandFromKey)) {
var commandFrom = toolMap[commandFromKey] as YamlNode;
if (commandFrom.value is String) {
command = [commandFrom.toString()];
if (command[0].isEmpty) {
throw DartdocOptionError(
'Tool commands must not be empty. Tool $name command entry '
'"$commandFrom" must contain at least one path.');
'"$commandFromKey" must contain at least one path.');
}
} else if (toolMap[commandFrom] is YamlList) {
command = (toolMap[commandFrom] as YamlList)
.map<String>((node) => node.toString())
.toList();
} else if (commandFrom is YamlList) {
command =
commandFrom.map<String>((node) => node.toString()).toList();
if (command.isEmpty) {
throw DartdocOptionError(
'Tool commands must not be empty. Tool $name command entry '
'"$commandFrom" must contain at least one path.');
'"$commandFromKey" must contain at least one path.');
}
} else {
throw DartdocOptionError(
'Tool commands must be a path to an executable, or a list of '
'strings that starts with a path to an executable. '
'The tool $name has a $commandFrom entry that is a '
'${toolMap[commandFrom].runtimeType}');
'The tool $name has a $commandFromKey entry that is a '
'${commandFrom.runtimeType}');
}
}
return command;
Expand Down Expand Up @@ -637,16 +637,17 @@ abstract class DartdocOption<T> {
_OptionValueWithContext<Object> valueWithContext, String missingFilename);

/// Call [_onMissing] for every path that does not exist.
void _validatePaths(_OptionValueWithContext<dynamic> valueWithContext) {
void _validatePaths(_OptionValueWithContext<Object> valueWithContext) {
if (!mustExist) return;
assert(isDir || isFile);
List<String> resolvedPaths;
if (valueWithContext.value is String) {
var value = valueWithContext.value;
if (value is String) {
resolvedPaths = [valueWithContext.resolvedValue];
} else if (valueWithContext.value is List<String>) {
resolvedPaths = valueWithContext.resolvedValue.toList();
} else if (valueWithContext.value is Map<String, String>) {
resolvedPaths = valueWithContext.resolvedValue.values.toList();
} else if (value is List<String>) {
resolvedPaths = valueWithContext.resolvedValue as List;
} else if (value is Map<String, String>) {
resolvedPaths = (valueWithContext.resolvedValue as Map).values.toList();
} else {
assert(
false,
Expand Down Expand Up @@ -1159,19 +1160,18 @@ abstract class _DartdocFileOption<T> implements DartdocOption<T> {
_OptionValueWithContext<Object> _valueAtFromFile(Folder dir) {
var yamlFileData = _yamlAtDirectory(dir);
var contextPath = yamlFileData.canonicalDirectoryPath;
dynamic yamlData = yamlFileData.data ?? {};
Object yamlData = yamlFileData.data ?? {};
for (var key in keys) {
if (!yamlData.containsKey(key)) return null;
yamlData = yamlData[key] ?? {};
if (yamlData is Map && !yamlData.containsKey(key)) return null;
yamlData = (yamlData as Map)[key] ?? {};
}

dynamic returnData;
Object returnData;
if (_isListString) {
if (yamlData is YamlList) {
returnData = <String>[];
for (var item in yamlData) {
returnData.add(item.toString());
}
returnData = [
for (var item in yamlData) item.toString(),
];
}
} else if (yamlData is YamlMap) {
// TODO(jcollins-g): This special casing is unfortunate. Consider
Expand Down Expand Up @@ -1739,7 +1739,7 @@ Future<List<DartdocOption<Object>>> createDartdocOptions(
help: 'Generate ONLY the docs for the Dart SDK.'),
DartdocOptionArgSynth<String>('sdkDir',
(DartdocSyntheticOption<String> option, Folder dir) {
if (!option.parent['sdkDocs'].valueAt(dir) &&
if (!(option.parent['sdkDocs'].valueAt(dir) as bool) &&
(option.root['topLevelPackageMeta'].valueAt(dir) as PackageMeta)
.requiresFlutter) {
String flutterRoot = option.root['flutterRoot'].valueAt(dir);
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,7 @@ abstract class ModelElement extends Canonicalization
}
if (params == null && element is FunctionTypedElement) {
if (_originalMember != null) {
params = (_originalMember as dynamic).parameters;
params = (_originalMember as FunctionTypedElement).parameters;
} else {
params = (element as FunctionTypedElement).parameters;
}
Expand Down
8 changes: 6 additions & 2 deletions lib/src/package_meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,12 @@ class _FilePackageMeta extends PubPackageMeta {

@override
bool get requiresFlutter =>
_pubspec['environment']?.containsKey('flutter') == true ||
_pubspec['dependencies']?.containsKey('flutter') == true;
_environment?.containsKey('flutter') == true ||
_dependencies?.containsKey('flutter') == true;

YamlMap /*?*/ get _environment => _pubspec['environment'];

YamlMap /*?*/ get _dependencies => _pubspec['environment'];

@override
File getReadmeContents() =>
Expand Down
24 changes: 12 additions & 12 deletions test/dartdoc_options_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void main() {
dartdocOptionSetSynthetic.add(
DartdocOptionSyntheticOnly<List<String>>('vegetableLoader',
(DartdocSyntheticOption<List<String>> option, Folder dir) {
if (option.root['mySpecialInteger'].valueAt(dir) > 20) {
if ((option.root['mySpecialInteger'].valueAt(dir) as num) > 20) {
return <String>['existing.dart'];
} else {
return <String>['not_existing.dart'];
Expand All @@ -81,7 +81,7 @@ void main() {
dartdocOptionSetSynthetic.add(
DartdocOptionArgSynth<String>('nonCriticalFileOption',
(DartdocSyntheticOption<String> option, Folder dir) {
return option.root['vegetableLoader'].valueAt(dir).first;
return (option.root['vegetableLoader'].valueAt(dir) as List).first;
}, resourceProvider, optionIs: OptionKind.file));

dartdocOptionSetFiles = DartdocOptionSet('dartdoc', resourceProvider);
Expand Down Expand Up @@ -637,13 +637,13 @@ dartdoc:
equals(
'Field dartdoc.fileOptionList from ${path.canonicalize(dartdocOptionsTwo.path)}, set to [existing.dart, thing/that/does/not/exist], resolves to missing path: '
'"${path.joinAll([
path.canonicalize(secondDir.path),
'thing',
'that',
'does',
'not',
'exist'
])}"'));
path.canonicalize(secondDir.path),
'thing',
'that',
'does',
'not',
'exist'
])}"'));
// It doesn't matter that this fails.
expect(dartdocOptionSetFiles['nonCriticalFileOption'].valueAt(firstDir),
equals(path.joinAll([path.canonicalize(firstDir.path), 'whatever'])));
Expand All @@ -663,9 +663,9 @@ dartdoc:
equals(
'Field dartdoc.fileOption from ${path.canonicalize(dartdocOptionsTwo.path)}, set to not existing, resolves to missing path: '
'"${path.joinAll([
path.canonicalize(secondDir.path),
"not existing"
])}"'));
path.canonicalize(secondDir.path),
"not existing"
])}"'));
});

test('DartdocOptionSetFile works for directory options', () {
Expand Down
2 changes: 1 addition & 1 deletion test/end2end/dartdoc_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void main() {
});

test('missing a required file path prints a fatal-error', () async {
var outputLines = [];
var outputLines = <String>[];
var impossiblePath = path.join(dartdocPath, 'impossible');
await expectLater(
() => subprocessLauncher.runStreamed(
Expand Down
15 changes: 9 additions & 6 deletions tool/doc_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,20 @@ Future<List<String>> _packageUrls(int page) {
return http
.get(Uri.parse('https://pub.dartlang.org/packages.json?page=$page'))
.then((response) {
return List<String>.from(json.decode(response.body)['packages']);
var decodedJson = json.decode(response.body) as Map;
return (decodedJson['packages'] as List).cast<String>();
});
}

Future<List<PackageInfo>> _getPackageInfos(List<String> packageUrls) {
var futures = packageUrls.map((String p) {
return http.get(Uri.parse(p)).then((response) {
var decodedJson = json.decode(response.body);
var decodedJson = json.decode(response.body) as Map;
String name = decodedJson['name'];
var versions = List<Version>.from(
decodedJson['versions'].map((v) => Version.parse(v)));
var versions = [
for (var version in decodedJson['versions'] as List)
Version.parse(version as String),
];
return PackageInfo(name, Version.primary(versions));
});
}).toList();
Expand Down Expand Up @@ -216,8 +219,8 @@ Future<void> _exec(String command, List<String> args,
});
}

bool _isOldSdkConstraint(var pubspecInfo) {
var environment = pubspecInfo['environment'];
bool _isOldSdkConstraint(Map<String, dynamic> pubspecInfo) {
var environment = pubspecInfo['environment'] as Map;
if (environment != null) {
var sdk = environment['sdk'];
if (sdk != null) {
Expand Down
14 changes: 6 additions & 8 deletions tool/grind.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:dartdoc/src/package_meta.dart';
import 'package:grinder/grinder.dart';
import 'package:path/path.dart' as path;
import 'package:yaml/yaml.dart' as yaml;
import 'package:yaml/yaml.dart';

import 'subprocess_launcher.dart';

Expand Down Expand Up @@ -402,7 +403,8 @@ WarningsCollection jsonMessageIterableToWarnings(Iterable<Map> messageIterable,
if (message.containsKey('level') &&
message['level'] == 'WARNING' &&
message.containsKey('data')) {
warningTexts.add(message['data']['text']);
var data = message['data'] as Map;
warningTexts.add(data['text']);
}
}
return warningTexts;
Expand Down Expand Up @@ -1001,15 +1003,11 @@ Future<void> checkChangelogHasVersion() async {

String _getPackageVersion() {
var pubspec = File('pubspec.yaml');
dynamic yamlDoc;
if (pubspec.existsSync()) {
yamlDoc = yaml.loadYaml(pubspec.readAsStringSync());
}
if (yamlDoc == null) {
if (!pubspec.existsSync()) {
fail('Cannot find pubspec.yaml in ${Directory.current}');
}
var version = yamlDoc['version'];
return version;
var yamlDoc = yaml.loadYaml(pubspec.readAsStringSync()) as YamlMap;
return yamlDoc['version'];
}

@Task('Rebuild generated files')
Expand Down
3 changes: 2 additions & 1 deletion tool/subprocess_launcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ class SubprocessLauncher {
if (result.containsKey('message')) {
line = result['message'];
} else if (result.containsKey('data')) {
line = result['data']['text'];
var data = result['data'] as Map;
line = data['text'];
}
}
return line.split('\n');
Expand Down

0 comments on commit 52e50c9

Please sign in to comment.