chore(supabase_lints): enable six DCM rules to guard new code, ignoring their intentional existing hits#1425
Merged
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.
…ores These rules each had one or two violations that are false positives or deliberate, so they are enabled with targeted inline ignores rather than code changes: - avoid-unassigned-fields: _streamFilter is declared on the base and assigned in the subclass. - avoid-passing-self-as-argument: re-auth on rejoin passes the socket's current token to its own setAuth, which requires the argument. - avoid-duplicate-constant-values: kindUserBroadcast (4) coincidentally equals userBroadcastMetaLength (4) but is a distinct protocol value. - avoid-unnecessary-type-assertions: the onError arity checks are real runtime validation; DCM mis-models function-type checks as always true. - match-getter-setter-field-names: a deprecated alias getter and a test fake's overridden getter. - function-always-returns-null: the public push() vestigial return type and a test mock method-call handler.
spydon
changed the title
…e-misused-test-matchers
…ingle-violation-rules
…e-misused-test-matchers
…ingle-violation-rules
spydon
changed the title
grdsdev
approved these changes
Jun 18, 2026
…iolation-rules # Conflicts: # packages/gotrue/test/src/constants_test.dart # packages/realtime_client/lib/src/realtime_channel.dart # packages/supabase_lints/lib/analysis_options.yaml
spydon
added a commit
that referenced
this pull request
Jun 18, 2026
Stacked on #1425. ## What Enables the `avoid-default-tostring` rule, which flags relying on `Object.toString()` (the unhelpful `Instance of 'X'`). All 18 sites were one of: - **Genuine fix** — `User.toString()` interpolated its `List<Factor>`, and `Factor` had no `toString`, so it rendered as `Instance of 'Factor'`. Added a `toString` to `Factor`. - **Enums in messages / data-class `toString`** — rendered with `.name` (e.g. `flowType: ${_flowType.name}`, `event: ${event.name}`) instead of relying on the default. Affects log lines and the `toString` of `AuthState`, `RealtimeSubscribeException`, `PostgresChangePayload`, `PresencePayload`, `PresenceSyncPayload`. No tests assert on these formats. - **Logic** — `convertCell` matched a `PostgresTypes` value via `e.toString() == 'PostgresTypes.$type'`; changed to `e.name == type` (equivalent, no default-`toString` reliance). - **Removed test** — `constants_test` had a test asserting `enum.toString()` returns the qualified name. That is Dart SDK behaviour, not this repo's to verify, so it was removed. ## Verification - `dcm analyze packages` -> no issues found. - `dart analyze` clean across affected packages (lib + test); `flutter analyze --fatal-warnings` clean. - Tests: gotrue constants_test 41, realtime transformers_test + socket_test 55, gotrue type unit tests pass. gotrue `test/src` count went 232 -> 231 (only the removed enum test); the 24 failures are pre-existing integration tests that need a local server, identical to baseline. ## Note Enums are rendered with `.name`, which drops the `EnumType.` prefix from the affected `toString`/log output (e.g. `signedIn` instead of `AuthChangeEvent.signedIn`). Nothing asserts on these strings.
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 #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.avoid-unassigned-fieldssupabase_stream_builder_streamFilterSupabaseStreamFilterBuildersubclass.avoid-passing-self-as-argumentrealtime_channelrejoinsetAuth, which requires the argument.avoid-duplicate-constant-valuesserializerkindUserBroadcast = 4userBroadcastMetaLength = 4; distinct protocol values that must not change.avoid-unnecessary-type-assertionspostgrest_builderthenonErrorarity checks are real runtime validation; DCM mis-models function-typeis!checks as always-true.match-getter-setter-field-namessnakeCase, lifecycle_test fake@Deprecatedalias getter and a test fake's overridden getter.function-always-returns-nullRealtimeClient.push, test mock handlerpushhas a vestigialString?return (changing tovoidwould be breaking); the other is a mock method-call handler.Verification
dcm analyze packages-> no issues found (including no unused-ignores).dart analyzeclean across affected packages (lib + test).flutter analyze --fatal-warningsclean on supabase_flutter.thenonError 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-nullString?return, left as-is here because changing it tovoidis a breaking public API change worth handling separately.