chore(supabase_lints): enable avoid-misused-test-matchers#1424
Merged
Conversation
Fix all existing violations and remove these rules from the shared ignore list so they now guard the codebase: - avoid-unnecessary-futures - avoid-unnecessary-local-late - avoid-unnecessary-type-casts - avoid-redundant-async - avoid-redundant-else - prefer-iterable-of - prefer-return-await - no-equal-then-else - avoid-misused-set-literals Most violations were resolved with dcm fix; the remaining ones were handled manually. The web isolate keeps returning a Future to match the IO implementation, and the IO isolate keeps a required late local that DCM flags as a false positive.
The avoid-unnecessary-futures cleanup turned the web and stub local storage helpers synchronous, but SharedPreferencesLocalStorage.persistSession returned that result directly from a non-async Future<void> method, which no longer type-checks. Make persistSession async and align the stub's hasAccessToken signature with the web implementation.
Enable and fix all violations of: - avoid-inferrable-type-arguments - prefer-declaring-const-constructor Most were resolved with dcm fix. Const constructors were added manually to the realtime presence payloads. Three map literals in realtime_client keep explicit type arguments (with inline ignores) because the inferred type would otherwise widen to a dynamic map and break compilation.
…ests The avoid-redundant-async cleanup stripped 'async' from the mock response closures in retry_test and stack_trace_test, so they returned StreamedResponse instead of the Future<StreamedResponse> their typedef requires and the files failed to compile. Wrap the responses in Future.value so they satisfy the typedef without the redundant async.
…blocked rules Enable and fix all violations of six low-risk rules: - prefer-null-aware-spread - avoid-only-rethrow - avoid-excessive-expressions - avoid-unnecessary-super - prefer-parentheses-with-if-null - avoid-unnecessary-local-variable Also split the ignore list so the rules whose suggested fixes need a newer language feature than our 3.4.0 floor (null-aware elements and dot shorthands) are grouped under their own section and cannot be enabled until the minimum SDK is raised.
…-fields SupabaseClient.dispose() now disposes the functions and rest clients and closes the auth HTTP client when the SDK created it (a user-provided client is left untouched). AuthHttpClient.close() forwards to its inner client so the internally created Client is actually torn down, fixing the leak dispose-class-fields was flagging, and the rule is enabled.
…tors Enabling prefer-declaring-const-constructor gave the test storage mocks const constructors, so flutter analyze --fatal-warnings flagged the call sites. Mark the invocations and the options they flow into as const.
…ispose-class-fields
Enable and fix all violations of: - avoid-map-keys-contains - avoid-missing-completer-stack-trace - avoid-nullable-tostring - avoid-unnecessary-return - avoid-unnecessary-late-fields completeError calls now pass a stack trace, the two SupabaseClient isolate fields drop late since they are set in the initializer list, and the nullable toString in postgrest_builder uses an explicit cast guarded by the preceding is-check.
…sconnect The subscription created for the websocket connection stream was never stored, so it could not be cancelled explicitly on disconnect. Store it and cancel it when the connection is torn down.
Fix all violations and enable the rule: - Wrap the expectLater targets in retry_test in closures so throwsA defers the call instead of matching the already-built future. - Replace vacuous expect(x, isNotNull) on non-nullable values with a meaningful assertion (isNotEmpty, count bounds, singleton identity) or remove it when the next line already asserts the value.
…e-misused-test-matchers
…e-misused-test-matchers
grdsdev
approved these changes
Jun 18, 2026
…test-matchers # Conflicts: # packages/realtime_client/test/channel_test.dart # packages/supabase_lints/lib/analysis_options.yaml
spydon
added a commit
that referenced
this pull request
Jun 18, 2026
…ng their intentional existing hits (#1425) Stacked on #1424. ## What Enables six DCM rules that each had only one or two violations. After inspection, every violation is a false positive or a deliberate pattern, so the rules are enabled with targeted inline `// ignore:` directives rather than code changes. This keeps new code covered by the rules while documenting the known exceptions. | Rule | Site | Why ignored | |------|------|-------------| | `avoid-unassigned-fields` | `supabase_stream_builder` `_streamFilter` | Declared on the base class, assigned in the `SupabaseStreamFilterBuilder` subclass. | | `avoid-passing-self-as-argument` | `realtime_channel` rejoin | Re-auth passes the socket's current token to its own `setAuth`, which requires the argument. | | `avoid-duplicate-constant-values` | `serializer` `kindUserBroadcast = 4` | Coincidentally equals `userBroadcastMetaLength = 4`; distinct protocol values that must not change. | | `avoid-unnecessary-type-assertions` | `postgrest_builder` `then` | The `onError` arity checks are real runtime validation; DCM mis-models function-type `is!` checks as always-true. | | `match-getter-setter-field-names` | gotrue `snakeCase`, lifecycle_test fake | A `@Deprecated` alias getter and a test fake's overridden getter. | | `function-always-returns-null` | `RealtimeClient.push`, test mock handler | Public `push` has a vestigial `String?` return (changing to `void` would be breaking); the other is a mock method-call handler. | ## Verification - `dcm analyze packages` -> no issues found (including no unused-ignores). - `dart analyze` clean across affected packages (lib + test). - `flutter analyze --fatal-warnings` clean on supabase_flutter. - Tests: postgrest stack_trace_test (exercises `then` onError validation) 5 pass, realtime socket_test 36 pass, supabase_flutter 49 pass. ## Note These are comment-only changes (inline ignores); no runtime behavior changes. The one arguably-improvable case is `RealtimeClient.push`'s always-null `String?` return, left as-is here because changing it to `void` is a breaking public API change worth handling separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1423.
What
Enables the
avoid-misused-test-matchersDCM rule by fixing all 31 flagged sites. Two patterns:throwsAon an already-evaluated builder (7 sites, retry_test)expectLater(client.from('users').select(), throwsA(...))passed the already-builtPostgrestBuildertothrowsA. Wrapped the targets in closures (() => client.from('users').select()) sothrowsAdefers the call. Behaviorally equivalent (the builder is lazy), and the correct idiom.Vacuous
expect(x, isNotNull)on non-nullable values (24 sites)The target types are non-nullable, so the assertion always passed. Each was either:
isNotEmpty(error message, user id, token, claims),greaterThanOrEqualTo(0)(count queries),same(Supabase.instance.client)(singleton identity).expect(res['type'], 'FeatureCollection')), or when it was a post-initializesmoke test whose value is "init did not throw".Verification
dcm analyze packages-> no issues found.dart analyzeclean across affected packages (lib + test).flutter analyze --fatal-warningsclean on supabase_flutter.