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

Jump to bottom

[gen_l10n] Throw error when arb file does not exist #107583

Merged
merged 4 commits into from Jul 15, 2022
Merged

[gen_l10n] Throw error when arb file does not exist #107583

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e271f96
init
thkim1011 Jul 13, 2022
2d12ef8
add test
thkim1011 Jul 15, 2022
fb5d232
better code :)
thkim1011 Jul 15, 2022
9346325
use stat once
thkim1011 Jul 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/flutter_tools/lib/src/localizations/gen_l10n.dart
View file
Open in desktop
Expand Up @@ -943,6 +943,12 @@ class LocalizationsGenerator {
static File templateArbFileFromFileName(String templateArbFileName, Directory inputDirectory) {
final File templateArbFile = inputDirectory.childFile(templateArbFileName);
final String templateArbFileStatModeString = templateArbFile.statSync().modeString();
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable isn't used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used in line 952 to check the permissions of the file, I think. I can put it right before that if-block if it makes it clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's all good, my mistake. See below.

final bool templateArbFileExists = templateArbFile.existsSync();
if (!templateArbFileExists) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just be if (!templateArbFile.existsSync())

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I was looking at the diffs not at the overall context. The version of the code you had originally, that just relied on one stat call made sense as it was. Sorry about the (my) confusion.

throw L10nException(
"The 'template-arb-file', $templateArbFile, does not exist."
);
}
if (templateArbFileStatModeString[0] == '-' && templateArbFileStatModeString[3] == '-') {
throw L10nException(
"The 'template-arb-file', $templateArbFile, is not readable.\n"
Expand Down
26 changes: 26 additions & 0 deletions packages/flutter_tools/test/general.shard/generate_localizations_test.dart
View file
Open in desktop
Expand Up @@ -180,6 +180,32 @@ void main() {
);
});

testWithoutContext('throws error when arb file does not exist', () {
// Set up project directory.
fs.currentDirectory
.childDirectory('lib')
.childDirectory('l10n')
.createSync(recursive: true);

// Arb file should be nonexistent in the l10n directory.
expect(
() => LocalizationsGenerator(
fileSystem: fs,
projectPathString: './',
inputPathString: defaultL10nPathString,
outputPathString: defaultL10nPathString,
templateArbFileName: defaultTemplateArbFileName,
outputFileString: defaultOutputFileString,
classNameString: defaultClassNameString,
),
throwsA(isA<L10nException>().having(
(L10nException e) => e.message,
'message',
contains(', does not exist.'),
)),
);
});

group('className should only take valid Dart class names', () {
setUp(() {
_standardFlutterDirectoryL10nSetup(fs);
Expand Down