Skip to content
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.

Sign up for GitHub

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

Display an error message instead of crashing on wrong data type in dartdoc_options.yaml #3372

Merged
merged 1 commit into from
Mar 23, 2023
Merged

Display an error message instead of crashing on wrong data type in dartdoc_options.yaml #3372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 17 additions & 3 deletions lib/src/dartdoc_options.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,17 @@ abstract class DartdocOption<T extends Object?> {

bool get _isDouble => _kDoubleVal is T;

String get _expectedTypeForDisplay {
if (_isString) return 'String';
if (_isListString) return 'list of Strings';
if (_isMapString) return 'map of String to String';
if (_isBool) return 'boolean';
if (_isInt) return 'int';
if (_isDouble) return 'double';
assert(false, 'Expecting an unknown type');
return '<<unknown>>';
}

final Map<String, _YamlFileData> __yamlAtCanonicalPathCache = {};

/// Implementation detail for [DartdocOptionFileOnly]. Make sure we use
Expand Down Expand Up @@ -945,9 +956,8 @@ abstract class _DartdocFileOption<T> implements DartdocOption<T> {
};
}
if (convertYamlToType == null) {
throw DartdocOptionError(
'Unable to convert yaml to type for option: $fieldName, method not '
'defined');
throw UnsupportedError(
'$fieldName: convertYamlToType method not defined');
}
var canonicalDirectoryPath =
resourceProvider.pathContext.canonicalize(contextPath);
Expand All @@ -964,6 +974,10 @@ abstract class _DartdocFileOption<T> implements DartdocOption<T> {
} else {
throw UnsupportedError('Type $T is not supported');
}
if (returnData == null) {
throw DartdocOptionError('Error in dartdoc_options.yaml, $fieldName: '
'expecting a $_expectedTypeForDisplay, got `$yamlData`');
}
return _OptionValueWithContext(returnData as T, contextPath,
definingFile: 'dartdoc_options.yaml');
}
Expand Down
130 changes: 72 additions & 58 deletions test/dartdoc_options_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class ConvertedOption {

void main() {
var resourceProvider = pubPackageMetaProvider.resourceProvider;
var path = pubPackageMetaProvider.resourceProvider.pathContext;

late final DartdocOptionRoot dartdocOptionSetFiles;
late final DartdocOptionRoot dartdocOptionSetArgs;
Expand Down Expand Up @@ -174,34 +175,30 @@ void main() {
));

tempDir = resourceProvider.createSystemTemp('options_test');
firstDir = resourceProvider
.getFolder(resourceProvider.pathContext.join(tempDir.path, 'firstDir'))
firstDir = resourceProvider.getFolder(path.join(tempDir.path, 'firstDir'))
..create();
firstExisting = resourceProvider.getFile(
resourceProvider.pathContext.join(firstDir.path, 'existing.dart'))
firstExisting = resourceProvider
.getFile(path.join(firstDir.path, 'existing.dart'))
..writeAsStringSync('');
secondDir = resourceProvider
.getFolder(resourceProvider.pathContext.join(tempDir.path, 'secondDir'))
secondDir = resourceProvider.getFolder(path.join(tempDir.path, 'secondDir'))
..create();
resourceProvider
.getFile(
resourceProvider.pathContext.join(secondDir.path, 'existing.dart'))
.getFile(path.join(secondDir.path, 'existing.dart'))
.writeAsStringSync('');

secondDirFirstSub = resourceProvider.getFolder(
resourceProvider.pathContext.join(secondDir.path, 'firstSub'))
secondDirFirstSub = resourceProvider
.getFolder(path.join(secondDir.path, 'firstSub'))
..create();
secondDirSecondSub = resourceProvider.getFolder(
resourceProvider.pathContext.join(secondDir.path, 'secondSub'))
secondDirSecondSub = resourceProvider
.getFolder(path.join(secondDir.path, 'secondSub'))
..create();

dartdocOptionsOne = resourceProvider.getFile(resourceProvider.pathContext
.join(firstDir.path, 'dartdoc_options.yaml'));
dartdocOptionsTwo = resourceProvider.getFile(resourceProvider.pathContext
.join(secondDir.path, 'dartdoc_options.yaml'));
var dartdocOptionsTwoFirstSub = resourceProvider.getFile(resourceProvider
.pathContext
.join(secondDirFirstSub.path, 'dartdoc_options.yaml'));
dartdocOptionsOne = resourceProvider
.getFile(path.join(firstDir.path, 'dartdoc_options.yaml'));
dartdocOptionsTwo = resourceProvider
.getFile(path.join(secondDir.path, 'dartdoc_options.yaml'));
var dartdocOptionsTwoFirstSub = resourceProvider
.getFile(path.join(secondDirFirstSub.path, 'dartdoc_options.yaml'));

dartdocOptionsOne.writeAsStringSync('''
dartdoc:
Expand Down Expand Up @@ -247,6 +244,42 @@ dartdoc:
}
});

group('invalid data in yaml', () {
late Folder invalid;
late File dartdocOptionsInvalid;

setUp(() {
invalid = resourceProvider.getFolder(path.join(tempDir.path, 'invalid'))
..create();
dartdocOptionsInvalid = resourceProvider
.getFile(path.join(invalid.path, 'dartdoc_options.yaml'));
dartdocOptionsInvalid.writeAsStringSync('''
dartdoc:
categoryOrder: 'options_one'
''');
});

tearDown(() {
try {
invalid.delete();
} catch (e) {
print('Ignoring error trying to delete temp: $e');
}
});

test('validate correct throw for wrong data type', () {
dartdocOptionSetFiles.parseArguments([]);
expect(() {
dartdocOptionSetFiles['categoryOrder'].valueAt(invalid);
},
throwsA(const TypeMatcher<DartdocOptionError>().having(
(e) => e.message,
'message',
equals('Error in dartdoc_options.yaml, dartdoc.categoryOrder: '
'expecting a list of Strings, got `options_one`'))));
});
});

group('new style synthetic option', () {
test('validate argument override changes value', () {
dartdocOptionSetSynthetic.parseArguments(['--my-special-integer', '12']);
Expand All @@ -272,10 +305,8 @@ dartdoc:
} on DartdocFileMissing catch (e) {
errorMessage = e.message;
}
var missingPath = resourceProvider.pathContext.canonicalize(
resourceProvider.pathContext.join(
resourceProvider.pathContext.absolute(tempDir.path),
'existing.dart'));
var missingPath = path.canonicalize(
path.join(path.absolute(tempDir.path), 'existing.dart'));
expect(
errorMessage,
equals('Synthetic configuration option vegetableLoaderChecked from '
Expand All @@ -296,7 +327,7 @@ dartdoc:
// Since this is an ArgSynth, it ignores the yaml option and resolves to the CWD
expect(
dartdocOptionSetSynthetic['nonCriticalFileOption'].valueAt(firstDir),
equals(resourceProvider.pathContext.canonicalize('stuff.zip')));
equals(path.canonicalize('stuff.zip')));
});

test('ArgSynth defaults to synthetic', () {
Expand All @@ -320,10 +351,8 @@ dartdoc:
} on DartdocFileMissing catch (e) {
errorMessage = e.message;
}
var missingPath = resourceProvider.pathContext.join(
resourceProvider.pathContext
.canonicalize(resourceProvider.pathContext.current),
'override-not-existing.dart');
var missingPath = path.join(
path.canonicalize(path.current), 'override-not-existing.dart');
expect(
errorMessage,
equals('Argument --file-option, set to override-not-existing.dart, '
Expand Down Expand Up @@ -386,20 +415,15 @@ dartdoc:
});

group('glob options', () {
String canonicalize(String path) =>
resourceProvider.pathContext.canonicalize(path);
String canonicalize(String inputPath) => path.canonicalize(inputPath);

test('work via the command line', () {
dartdocOptionSetAll
.parseArguments(['--glob-option', p.join('foo', '**')]);
expect(
dartdocOptionSetAll['globOption'].valueAtCurrent(),
equals([
resourceProvider.pathContext.joinAll([
canonicalize(resourceProvider.pathContext.current),
'foo',
'**'
])
path.joinAll([canonicalize(path.current), 'foo', '**'])
]));
});

Expand All @@ -408,25 +432,21 @@ dartdoc:
expect(
dartdocOptionSetAll['globOption'].valueAt(secondDir),
equals([
canonicalize(
resourceProvider.pathContext.join(secondDir.path, 'q*.html')),
canonicalize(
resourceProvider.pathContext.join(secondDir.path, 'e*.dart')),
canonicalize(path.join(secondDir.path, 'q*.html')),
canonicalize(path.join(secondDir.path, 'e*.dart')),
]));
// No child override, should be the same as parent
expect(
dartdocOptionSetAll['globOption'].valueAt(secondDirSecondSub),
equals([
canonicalize(
resourceProvider.pathContext.join(secondDir.path, 'q*.html')),
canonicalize(
resourceProvider.pathContext.join(secondDir.path, 'e*.dart')),
canonicalize(path.join(secondDir.path, 'q*.html')),
canonicalize(path.join(secondDir.path, 'e*.dart')),
]));
// Child directory overrides
expect(
dartdocOptionSetAll['globOption'].valueAt(secondDirFirstSub),
equals([
resourceProvider.pathContext.joinAll(
path.joinAll(
[canonicalize(secondDirFirstSub.path), '**', '*.dart'])
]));
});
Expand Down Expand Up @@ -464,10 +484,8 @@ dartdoc:
} on DartdocFileMissing catch (e) {
errorMessage = e.message;
}
var missingPath = resourceProvider.pathContext.join(
resourceProvider.pathContext
.canonicalize(resourceProvider.pathContext.current),
'not_found.txt');
var missingPath =
path.join(path.canonicalize(path.current), 'not_found.txt');
expect(
errorMessage,
equals('Argument --single-file, set to not_found.txt, resolves to '
Expand All @@ -478,7 +496,7 @@ dartdoc:
String? errorMessage;
dartdocOptionSetArgs.parseArguments([
'--files-flag',
resourceProvider.pathContext.absolute(firstExisting.path),
path.absolute(firstExisting.path),
'--files-flag',
'other_not_found.txt'
]);
Expand All @@ -487,14 +505,12 @@ dartdoc:
} on DartdocFileMissing catch (e) {
errorMessage = e.message;
}
var missingPath = resourceProvider.pathContext.join(
resourceProvider.pathContext
.canonicalize(resourceProvider.pathContext.current),
'other_not_found.txt');
var missingPath =
path.join(path.canonicalize(path.current), 'other_not_found.txt');
expect(
errorMessage,
equals('Argument --files-flag, set to '
'[${resourceProvider.pathContext.absolute(firstExisting.path)}, '
'[${path.absolute(firstExisting.path)}, '
'other_not_found.txt], resolves to missing path: '
'"$missingPath"'));
});
Expand All @@ -506,10 +522,8 @@ dartdoc:
.parseArguments(['--unimportant-file', 'this-will-never-exist']);
expect(
dartdocOptionSetArgs['unimportantFile'].valueAt(tempDir),
equals(resourceProvider.pathContext.join(
resourceProvider.pathContext
.canonicalize(resourceProvider.pathContext.current),
'this-will-never-exist')));
equals(path.join(
path.canonicalize(path.current), 'this-will-never-exist')));
});

test('DartdocOptionArgOnly has correct flag names', () {
Expand Down