feat: add rule for `no_force_unwrap` #272

ostk0069 · 2025-03-05T14:26:12Z

Overview

Adding new rules for force unwrap value

Motivation

Dart analyzer system is not smart enough with force unwrap useless.
(eg. does not recognize force unwrap inside if block)
So that developers would feel more close to adding force unwrap.

Feature type

Lint Rule
Quick fix
Assist
Other (Update docs, Configure CI, etc...)

Checklist

I have read the CONTRIBUTING docs.
I have written an related issue or linked with existing issues.
I have written tests and passed all of them.
- Use melos test to run tests.
I have updated docs (if needed).

gemini-code-assist

Hello @ostk0069, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request introduces a new lint rule, no_force_unwrap, to discourage the use of the force unwrap operator ! in Dart code. The rule suggests using null coalescing or pattern matching as safer alternatives. The PR includes the implementation of the lint rule, along with quick fixes to replace the force unwrap operator with either a null coalescing operator or pattern matching (if Dart version is 3.0.0 or higher).

Highlights

New Lint Rule: no_force_unwrap: Adds a new lint rule that flags the use of the force unwrap operator !.
Quick Fixes: Provides quick fixes to replace force unwrapping with null coalescing (??) or pattern matching (if (x case final value) return value).
Dart Version Compatibility: The pattern matching quick fix is only available for Dart versions 3.0.0 and above.

Changelog

packages/nilts/lib/nilts.dart
- Added import for the new NoForceUnwrap lint rule.
- Registered the NoForceUnwrap lint rule within the _NiltsLint class, making it available for analysis.
packages/nilts/lib/src/change_priority.dart
- Added priority constants for the new quick fixes: addNullCoalescingOperator and addPatternMatching.
packages/nilts/lib/src/lints/no_force_unwrap.dart
- Created a new file containing the implementation of the NoForceUnwrap lint rule.
- Implemented the NoForceUnwrap class, which extends DartLintRule and defines the logic for detecting force unwrap operators.
- Implemented quick fixes _AddNullCoalescingOperator and _ReplaceWithPatternMatching to suggest alternative approaches.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Trivia time!

What is the null coalescing operator in Dart, and what does it do?

Click here for the answer

The null coalescing operator in Dart is `??`. It returns the expression on the left if it is not null; otherwise, it evaluates and returns the expression on the right.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new lint rule, no_force_unwrap, which aims to prevent the use of the ! operator for force unwrapping in Dart code. The rule suggests using null coalescing or pattern matching as safer alternatives. The implementation includes a lint rule and quick fixes. Overall, this is a valuable addition to the linter.

Summary of Findings

Missing Default Value in Null Coalescing Fix: The _AddNullCoalescingOperator fix suggests replacing the force unwrap with a null coalescing operator but uses defaultValue without defining it. This could lead to confusion or errors if the user doesn't know what to replace it with.

Merge Readiness

The pull request is almost ready for merging. The no_force_unwrap lint rule and its associated quick fixes seem to be implemented correctly. However, the _AddNullCoalescingOperator fix needs to be updated to provide a more concrete example or guidance on what to use as the default value. Once this is addressed, the pull request should be good to merge. I am unable to approve this pull request, and recommend that others review and approve this code before merging.

packages/nilts/lib/src/lints/no_force_unwrap.dart

ronnnnn · 2025-03-10T15:11:16Z

packages/nilts/lib/nilts.dart

@@ -43,5 +44,6 @@ class _NiltsLint extends PluginBase {
        const ShrinkWrappedScrollView(),
        if (_dartVersion >= const DartVersion(major: 3, minor: 0, patch: 0))
          UnnecessaryRebuildsFromMediaQuery(_dartVersion),
+        NoForceUnwrap(_dartVersion),


Sort with alphabetical order for readability.

ronnnnn · 2025-03-10T15:13:53Z

packages/nilts/lib/src/lints/no_force_unwrap.dart

+  @override
+  List<Fix> getFixes() => [
+        _AddNullCoalescingOperator(),
+        if (_dartVersion >= const DartVersion(major: 3, minor: 0, patch: 0))


An unnecessary condition check because nilts supports Dart SDK >=3.0.0.

https://github.com/dassssshers/nilts/blob/main/packages/nilts/pubspec.yaml#L27

ronnnnn · 2025-03-10T15:17:18Z

packages/nilts/README.md

@@ -115,6 +115,7 @@ Some of lint rules support quick fixes on IDE.
 | [no_support_web_platform_check](#no_support_web_platform_check)                 | Checks if `Platform.isXxx` usages.                                             |  Any versions nilts supports   | Practice  |  Experimental  |    ✅️    |
 | [shrink_wrapped_scroll_view](#shrink_wrapped_scroll_view)                       | Checks the content of the scroll view is shrink wrapped.                       |  Any versions nilts supports   | Practice  |  Experimental  |    ✅️    |
 | [unnecessary_rebuilds_from_media_query](#unnecessary_rebuilds_from_media_query) | Checks `MediaQuery.xxxOf(context)` or `MediaQuery.maybeXxxOf(context)` usages. | >= Flutter 3.10.0 (Dart 3.0.0) | Practice  |  Experimental  |    ✅️    |
+| [no_force_unwrap](#no_force_unwrap)                                             | Checks usage of the `!` operator for forced unwrapping.                        |  Any versions nilts supports   | Practice  |  Experimental  |    ✅️    |


Lint rule names should describe what is problem in nilts.
no_force_unwrap represents how to avoid problem.

https://github.com/dassssshers/nilts/blob/main/CONTRIBUTING.md#lint-name

I think it's confusing how 'unwrap' and 'wrap' are often used with Widgets, don't you think?
'cast' may be better.

ronnnnn · 2025-03-10T15:18:46Z

packages/nilts/README.md

+<!-- prettier-ignore-start -->
+```dart
+if (someValue case final actualValue) {
+  print('取得した値: $actualValue');


Writing in English is better.

ronnnnn · 2025-03-10T15:23:37Z

packages/nilts/lib/src/lints/no_force_unwrap.dart

+          .addDartFileEdit((builder) {
+        builder.addSimpleReplacement(
+          node.sourceRange,
+          'if (${node.operand} case final value) return value',


Type definition may be needed between final and value.

// compile error int length() { if (nullableString case final value) { return value.length; } } // ok int length() { if (nullableString case final String value) { return value.length; } return 0; }

ronnnnn · 2025-03-10T15:39:53Z

packages/nilts/lib/src/lints/no_force_unwrap.dart

+          .addDartFileEdit((builder) {
+        builder.addSimpleReplacement(
+          node.sourceRange,
+          '${node.operand} ?? /* Replace with a suitable default value */',


Surround with () and comments may not be needed (it's too long to delete).

Suggested change

'${node.operand} ?? /* Replace with a suitable default value */',

'(${node.operand} ?? )',

gemini-code-assist bot reviewed Mar 5, 2025

View reviewed changes

github-actions bot assigned ostk0069 Mar 5, 2025

gemini-code-assist bot suggested changes Mar 5, 2025

View reviewed changes

packages/nilts/lib/src/lints/no_force_unwrap.dart Outdated Show resolved Hide resolved

ostk0069 force-pushed the feat/no_force_unwrap branch from 934f263 to f3fd641 Compare March 9, 2025 09:10

feat: add rule for no_force_unwrap

61b896d

ostk0069 force-pushed the feat/no_force_unwrap branch from f3fd641 to 61b896d Compare March 9, 2025 09:11

ostk0069 marked this pull request as ready for review March 9, 2025 15:32

ostk0069 requested a review from ronnnnn as a code owner March 9, 2025 15:32

ronnnnn reviewed Mar 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add rule for `no_force_unwrap` #272

feat: add rule for `no_force_unwrap` #272

ostk0069 commented Mar 5, 2025 •

edited

Loading

gemini-code-assist bot left a comment

gemini-code-assist bot left a comment

ronnnnn Mar 10, 2025

ronnnnn Mar 10, 2025

ronnnnn Mar 10, 2025

ronnnnn Mar 10, 2025

ronnnnn Mar 10, 2025

ronnnnn Mar 10, 2025

ronnnnn Mar 10, 2025

	'${node.operand} ?? /* Replace with a suitable default value */',
	'(${node.operand} ?? )',

feat: add rule for no_force_unwrap #272

Are you sure you want to change the base?

feat: add rule for no_force_unwrap #272

Conversation

ostk0069 commented Mar 5, 2025 • edited Loading

Overview

Motivation

Feature type

Checklist

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

ronnnnn Mar 10, 2025

Choose a reason for hiding this comment

feat: add rule for `no_force_unwrap` #272

feat: add rule for `no_force_unwrap` #272

ostk0069 commented Mar 5, 2025 •

edited

Loading