amplify-codegen
amplify-codegen copied to clipboard
Datastore codegen created models don't respect Dart linting rules
Describe the bug CLI codegen models creates QueryFields that use uppercased variables.
Example:
static final QueryField SESSIONID = QueryField(fieldName: "sessionID");
They get picked up by the IDE as
Name non-constant identifiers using lowerCamelCase.dart(non_constant_identifier_names)
To Reproduce Steps to reproduce the behavior:
- Create a graphql schema
- Run amplify codegen models
- Scroll down to 'static final QueryField.'
- See the IDE screaming for non_constant_identifier_names errors
Expected behavior Respecting dart listing rules
Platform Amplify Flutter current supports iOS and Android. This issue is reproducible in (check all that apply): [X] Android [X] iOS
Amplify CLI version 4.45.2
Output of flutter doctor -v
[!] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
! Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses
[✓] Xcode - develop for iOS and macOS
[✓] Chrome - develop for the web
[✓] Android Studio (version 4.1)
[✓] VS Code (version 1.54.3)
[✓] Connected device (3 available)```
</details>
Re-adding my wish list from aws-amplify/amplify-flutter#241.
Wish list
- Generated model code should at least pass all pedantic linter rules and not rely on implicit or dynamic casts. Other more strict options welcome (e.g. lint rules).
- Generated filenames should match Effective Dart Style.
Turn something like this on for the amplify-flutter repo and codegen generated models and see what you get for warning/errors.
include: package:lint/analysis_options.yaml
analyzer:
strong-mode:
implicit-casts: false
implicit-dynamic: false
errors:
missing_return: error
missing_required_param: error
must_be_immutable: error
invalid_use_of_protected_member: error
dead_code: info
sdk_version_async_exported_from_core: ignore
parameter_assignments: error
exclude:
# This next line will need to be uncommented to exclude the generated models.
# - lib/models/*.dart
linter:
rules:
- unnecessary_statements
- unnecessary_lambdas
- avoid_classes_with_only_static_members
- avoid_renaming_method_parameters
- camel_case_types
- constant_identifier_names
- cascade_invocations
- omit_local_variable_types
- sort_constructors_first
- sort_unnamed_constructors_first
- prefer_single_quotes
- directives_ordering
- no_default_cases
Hello @cto-leaps and @kjones - Thank you for your input. This is something that we are looking into.
I think this can be broken down into two issues:
- The code generated models are not excluded from lint rules defined within the project. This is an issue since the generated models cannot possibly respect every possible lint rule that may be used within a project. To address this, the models probably need to be excluded from analysis in analysis_options.yaml.
- The code generated models are not following dart/flutter conventions. With Flutter 2.5.0+, at least some of these conventions are now being enforced in new projects with flutter_lints. If code generated models were excluded from a projects lint rules, this wouldn't be as apparent. However, the generated models should still ideally follow conventions.
At least as a temporary workaround for those facing analysis options with generated models, the following snippet can be added to analysis_options.yaml.
analyzer:
exclude:
- "lib/models/*"
A new version of amplify-codegen has been released with the change from aws-amplify/amplify-codegen#248.
A few updates now that https://github.com/aws-amplify/amplify-codegen/pull/248 is merged.
This PR added ignores for all the rules from flutter_lints that the codegen models were not following. The goal will be to have the codegen models actually follow these rules in the future. For now, this will prevent lint issues from showing up for projects using flutter_lints. We will keep this issue open to track the work to have the models actually follow the rules.
The code generated models are not excluded from lint rules defined within the project
This was discussed a bit with the amplify-flutter team and we decided that it doesn't make sense to try to modify a projects analysis_options.yaml. Automatically updating the analysis_options.yaml file isn't very straightforward. Additionally, we ultimately want the codegen models follow flutter_lints (or a stricter rule set), so updating the files will not be needed if the project uses flutter_lints. If a project is using custom rules, the exclusion can always be added manually.
I am going to transfer this issue to aws-amplify/amplify-codegen as it is a more appropriate location.
The remaining work is to update the generated code to follow lint rules by flutter_lints so that the // ignore_for_file: ...
can be removed from the generated code without causing lint issues for most customers.
As mentioned in https://github.com/aws-amplify/amplify-flutter/issues/902#issuecomment-1464364229, we may want to consider lint rules from popular linting packages (for example, very_good_analysis).
In the short term, we could ignore rules from these packages. In the long term, we can attempt to follow these rules.