From 9cdd343ca70982e361e053187f688f3e6932e26a Mon Sep 17 00:00:00 2001 From: Rob Becker Date: Tue, 15 Sep 2026 22:38:23 -0600 Subject: [PATCH] refactor(models): strengthen model nullability and enable strict analyzer settings Co-authored-by: Cursor --- analysis_options.yaml | 4 +- example/languages.dart | 6 +-- example/user_info.dart | 2 +- lib/src/common/model/checks.dart | 63 ++++++++++++------------ lib/src/common/model/misc.dart | 2 +- lib/src/common/model/reaction.dart | 7 ++- lib/src/common/model/repos.dart | 7 ++- lib/src/common/model/repos_releases.dart | 2 +- lib/src/common/model/users.dart | 2 +- lib/src/common/util/oauth2.dart | 7 ++- lib/src/common/util/pagination.dart | 13 +++-- lib/src/common/xplat_common.dart | 13 ++--- test/common/github_test.dart | 9 ++-- test/data_object_test.dart | 2 +- test/experiment/org_hooks.dart | 2 +- test/experiment/orglist.dart | 2 +- test/helper/http.dart | 8 +-- test/scenarios_test.dart | 7 +-- test/unit/checks_test.dart | 14 ++++-- test/unit/checksuite_test.dart | 9 ++-- test/unit/common/model/misc_test.dart | 4 +- test/unit/common/model/pulls_test.dart | 3 +- test/unit/issues_test.dart | 4 +- tool/process_github_schema.dart | 3 +- 24 files changed, 113 insertions(+), 82 deletions(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index f2974469..a94d8108 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -2,7 +2,9 @@ include: package:lints/recommended.yaml analyzer: language: - #strict-casts: true + strict-casts: true + strict-inference: true + strict-raw-types: true linter: rules: diff --git a/example/languages.dart b/example/languages.dart index aa8b7ec1..faf665ae 100644 --- a/example/languages.dart +++ b/example/languages.dart @@ -52,11 +52,11 @@ String generateMarkdown(int accuracy) { |Name|Bytes|Percentage| |-----|-----|-----| '''); - data.sort((a, b) => b[1].compareTo(a[1])); + data.sort((a, b) => (b[1] as num).compareTo(a[1] as num)); for (final info in data) { - final String? name = info[0]; - final int bytes = info[1]; + final name = info[0] as String?; + final bytes = info[1] as int; final num percentage = (bytes / total) * 100; md.writeln('|$name|$bytes|${percentage.toStringAsFixed(accuracy)}|'); } diff --git a/example/user_info.dart b/example/user_info.dart index 656207b1..b0cce403 100644 --- a/example/user_info.dart +++ b/example/user_info.dart @@ -53,7 +53,7 @@ void loadUser() { append('Created', user.createdAt); document.getElementById('load')!.hidden = true; document.getElementById('token')!.hidden = true; - }).catchError((e) { + }).catchError((Object e) { if (e is AccessForbidden) { window.alert('Invalid Token'); } diff --git a/lib/src/common/model/checks.dart b/lib/src/common/model/checks.dart index b7666579..e5852953 100644 --- a/lib/src/common/model/checks.dart +++ b/lib/src/common/model/checks.dart @@ -20,8 +20,7 @@ class CheckRunAnnotationLevel extends EnumWithValue { case 'failure': return failure; default: - throw Exception( - 'This level of check run annotation is unimplemented: $value.'); + return CheckRunAnnotationLevel._(value ?? 'notice'); } } @@ -70,8 +69,7 @@ class CheckRunConclusion extends EnumWithValue { return level; } } - throw Exception( - 'This level of check run conclusion is unimplemented: $value.'); + return CheckRunConclusion._(value); } } @@ -124,16 +122,19 @@ class CheckRun { break; } } + final checkSuite = input['check_suite'] as Map?; return CheckRun._( - name: input['name'], - id: input['id'], - externalId: input['external_id'], + name: input['name'] as String?, + id: input['id'] as int?, + externalId: input['external_id'] as String?, status: status, - headSha: input['head_sha'], - checkSuiteId: input['check_suite']['id'], - detailsUrl: input['details_url'], - startedAt: DateTime.parse(input['started_at']), - conclusion: CheckRunConclusion._fromValue(input['conclusion']), + headSha: input['head_sha'] as String?, + checkSuiteId: checkSuite?['id'] as int?, + detailsUrl: input['details_url'] as String?, + startedAt: input['started_at'] != null + ? (DateTime.tryParse(input['started_at'] as String) ?? DateTime.now()) + : DateTime.now(), + conclusion: CheckRunConclusion._fromValue(input['conclusion'] as String?), ); } @@ -143,7 +144,7 @@ class CheckRun { 'id': id, 'external_id': externalId, 'status': status, - 'head_sha': externalId, + 'head_sha': headSha, 'check_suite': { 'id': checkSuiteId, }, @@ -273,16 +274,16 @@ class CheckRunAnnotation { factory CheckRunAnnotation.fromJSON(Map input) { return CheckRunAnnotation( - path: input['path'], - startLine: input['start_line'], - endLine: input['end_line'], - startColumn: input['start_column'], - endColumn: input['end_column'], - annotationLevel: - CheckRunAnnotationLevel._fromValue(input['annotation_level']), - title: input['title'], - message: input['message'], - rawDetails: input['raw_details'], + path: input['path'] as String, + startLine: input['start_line'] as int, + endLine: input['end_line'] as int, + startColumn: input['start_column'] as int?, + endColumn: input['end_column'] as int?, + annotationLevel: CheckRunAnnotationLevel._fromValue( + input['annotation_level'] as String?), + title: (input['title'] as String?) ?? '', + message: (input['message'] as String?) ?? '', + rawDetails: (input['raw_details'] ?? input['rax_details']) as String?, ); } @@ -296,7 +297,7 @@ class CheckRunAnnotation { 'annotation_level': annotationLevel.toString(), 'message': message, 'title': title, - 'rax_details': rawDetails, + 'raw_details': rawDetails, }); } } @@ -376,16 +377,16 @@ class CheckSuite { }); factory CheckSuite.fromJson(Map input) { - var pullRequestsJson = input['pull_requests'] as List; + var pullRequestsJson = (input['pull_requests'] as List?) ?? const []; var pullRequests = pullRequestsJson .map((dynamic json) => PullRequest.fromJson(json as Map)) .toList(); return CheckSuite( - conclusion: CheckRunConclusion._fromValue(input['conclusion']), - headBranch: input['head_branch'], - headSha: input['head_sha'], - id: input['id'], + conclusion: CheckRunConclusion._fromValue(input['conclusion'] as String?), + headBranch: input['head_branch'] as String?, + headSha: input['head_sha'] as String?, + id: input['id'] as int?, pullRequests: pullRequests, ); } @@ -414,8 +415,8 @@ class AutoTriggerChecks { factory AutoTriggerChecks.fromJson(Map input) { return AutoTriggerChecks( - appId: input['app_id'], - setting: input['setting'], + appId: input['app_id'] as int, + setting: input['setting'] as bool?, ); } diff --git a/lib/src/common/model/misc.dart b/lib/src/common/model/misc.dart index 18fc0a54..7ac457d7 100644 --- a/lib/src/common/model/misc.dart +++ b/lib/src/common/model/misc.dart @@ -51,7 +51,7 @@ class RateLimit { final remaining = rateJson?['remaining'] as int?; final resets = rateJson?['reset'] == null ? null - : DateTime.fromMillisecondsSinceEpoch(rateJson?['reset']); + : DateTime.fromMillisecondsSinceEpoch(rateJson!['reset'] as int); return RateLimit(limit, remaining, resets); } diff --git a/lib/src/common/model/reaction.dart b/lib/src/common/model/reaction.dart index a9b73d7f..2eacb82e 100644 --- a/lib/src/common/model/reaction.dart +++ b/lib/src/common/model/reaction.dart @@ -70,7 +70,12 @@ class ReactionType { ':eyes:': eyes, }; - static ReactionType? fromString(String? content) => _types[content!]; + static ReactionType? fromString(String? content) { + if (content == null) { + return null; + } + return _types[content] ?? ReactionType._(content, content); + } } @JsonSerializable() diff --git a/lib/src/common/model/repos.dart b/lib/src/common/model/repos.dart index d09c1a7e..3382106c 100644 --- a/lib/src/common/model/repos.dart +++ b/lib/src/common/model/repos.dart @@ -684,9 +684,12 @@ class LanguageBreakdown { /// The Primary Language String get primary { - final list = mapToList(_data); + if (_data.isEmpty) { + return ''; + } + final list = _data.entries.toList(); list.sort((a, b) { - return a.value.compareTo(b.value); + return b.value.compareTo(a.value); }); return list.first.key; } diff --git a/lib/src/common/model/repos_releases.dart b/lib/src/common/model/repos_releases.dart index 30a80172..0feffd9a 100644 --- a/lib/src/common/model/repos_releases.dart +++ b/lib/src/common/model/repos_releases.dart @@ -86,7 +86,7 @@ class Release { /// Release Assets List? assets; - List? errors; + List? errors; factory Release.fromJson(Map input) => _$ReleaseFromJson(input); diff --git a/lib/src/common/model/users.dart b/lib/src/common/model/users.dart index f66fd395..0df17bcb 100644 --- a/lib/src/common/model/users.dart +++ b/lib/src/common/model/users.dart @@ -43,7 +43,7 @@ class User { }); @JsonKey(includeToJson: false, includeFromJson: false) - Map? json; // TODO remove + Map? json; // TODO remove /// User's Username String? login; diff --git a/lib/src/common/util/oauth2.dart b/lib/src/common/util/oauth2.dart index 9333d606..dd98cc61 100644 --- a/lib/src/common/util/oauth2.dart +++ b/lib/src/common/util/oauth2.dart @@ -87,8 +87,11 @@ class OAuth2Flow { if (json['error'] != null) { throw Exception(json['error']); } - return ExchangeResponse(json['access_token'], json['token_type'], - (json['scope'] as String).split(',')); + return ExchangeResponse( + json['access_token'] as String?, + json['token_type'] as String?, + ((json['scope'] as String?) ?? '').split(','), + ); }); } } diff --git a/lib/src/common/util/pagination.dart b/lib/src/common/util/pagination.dart index 93f3a0d7..b53aa623 100644 --- a/lib/src/common/util/pagination.dart +++ b/lib/src/common/util/pagination.dart @@ -41,7 +41,7 @@ class PaginationHelper { if (serverErrors >= maxServerErrors) { break; } - await Future.delayed(serverErrorBackOff); + await Future.delayed(serverErrorBackOff); continue; } @@ -98,12 +98,15 @@ class PaginationHelper { body: body, statusCode: statusCode, )) { + final decoded = jsonDecode(response.body); final json = arrayKey == null - ? jsonDecode(response.body) as List? - : (jsonDecode(response.body) as Map)[arrayKey]; + ? (decoded as List?) + : ((decoded as Map)[arrayKey] as List?); - for (final item in json) { - yield (item as T?)!; + if (json != null) { + for (final item in json) { + yield (item as T?)!; + } } } } diff --git a/lib/src/common/xplat_common.dart b/lib/src/common/xplat_common.dart index 1c60f906..16a51a20 100644 --- a/lib/src/common/xplat_common.dart +++ b/lib/src/common/xplat_common.dart @@ -2,28 +2,29 @@ import 'package:github/src/common.dart'; /// Looks for GitHub Authentication information from the current environment. /// -/// If in a browser context, it will look through query string parameters first -/// and then sessionStorage. +/// If in a browser context, it will check sessionStorage (and optionally query strings +/// if [allowQueryAuth] is explicitly set to true, which is deprecated). /// /// If in a server, command line or Flutter context it will use the system environment variables. /// /// In both contexts it delegates to [findAuthenticationInMap] to find the /// github token or username and password. -Authentication findAuthenticationFromEnvironment() => +Authentication findAuthenticationFromEnvironment( + {bool allowQueryAuth = false}) => const Authentication.anonymous(); /// Checks the passed in map for keys in [COMMON_GITHUB_TOKEN_ENV_KEYS]. /// The first one that exists is used as the github token to call [Authentication.withToken] with. /// If the above fails, the GITHUB_USERNAME and GITHUB_PASSWORD keys will be checked. /// If those keys both exist, then [Authentication.basic] will be used. -Authentication? findAuthenticationInMap(Map map) { +Authentication? findAuthenticationInMap(Map map) { for (final key in COMMON_GITHUB_TOKEN_ENV_KEYS) { if (map.containsKey(key)) { - return Authentication.withToken(map[key]); + return Authentication.withToken(map[key] as String?); } if (map['GITHUB_USERNAME'] is String && map['GITHUB_PASSWORD'] is String) { return Authentication.basic( - map['GITHUB_USERNAME'], map['GITHUB_PASSWORD']); + map['GITHUB_USERNAME'] as String?, map['GITHUB_PASSWORD'] as String?); } } return null; diff --git a/test/common/github_test.dart b/test/common/github_test.dart index 97ce2930..326fe5d7 100644 --- a/test/common/github_test.dart +++ b/test/common/github_test.dart @@ -15,7 +15,8 @@ void main() { }); final github = GitHub(client: client); - await github.getJSON(''); // Make HTTP request + await github.getJSON, Map>( + ''); // Make HTTP request expect(request, isNotNull); expect(request!.headers.containsKey(GitHub.versionHeader), isTrue); @@ -31,7 +32,8 @@ void main() { }); final github = GitHub(client: client); - await github.getJSON(''); // Make HTTP request + await github.getJSON, Map>( + ''); // Make HTTP request expect(request, isNotNull); expect(request!.headers.containsKey('User-Agent'), isTrue); @@ -50,7 +52,8 @@ void main() { client: client, auth: const Authentication.anonymous(), ); - await github.getJSON(''); // Make HTTP request + await github.getJSON, Map>( + ''); // Make HTTP request expect(request, isNotNull); expect(request!.headers.containsKey('Authorization'), isFalse); diff --git a/test/data_object_test.dart b/test/data_object_test.dart index f1d1d5db..df5607ba 100644 --- a/test/data_object_test.dart +++ b/test/data_object_test.dart @@ -41,4 +41,4 @@ void main() { }); } -String _prettyEncode(obj) => GitHubJson.encode(obj, indent: ' '); +String _prettyEncode(Object obj) => GitHubJson.encode(obj, indent: ' '); diff --git a/test/experiment/org_hooks.dart b/test/experiment/org_hooks.dart index 9e0d79f8..0598a014 100644 --- a/test/experiment/org_hooks.dart +++ b/test/experiment/org_hooks.dart @@ -1,7 +1,7 @@ import 'dart:async'; import '../helper.dart'; -Future main() async { +Future main() async { const org = 'IOT-DSA'; final hooks = await github.organizations.listHooks(org).toList(); diff --git a/test/experiment/orglist.dart b/test/experiment/orglist.dart index 01aa4c6c..d0a18105 100644 --- a/test/experiment/orglist.dart +++ b/test/experiment/orglist.dart @@ -1,7 +1,7 @@ import 'dart:async'; import 'package:github/github.dart'; -Future main() async { +Future main() async { final github = GitHub(); final repos = await github.repositories.listUserRepositories('dart-lang').toList(); diff --git a/test/helper/http.dart b/test/helper/http.dart index ffed40d4..8b576526 100644 --- a/test/helper/http.dart +++ b/test/helper/http.dart @@ -32,16 +32,16 @@ class MockResponse extends http.Response { final responseData = jsonDecode(asset('responses/$name.json').readAsStringSync()) as Map; - final headers = responseData['headers'] as Map; + final headers = (responseData['headers'] as Map).cast(); final dynamic body = responseData['body']; - final int statusCode = responseData['statusCode']; + final statusCode = responseData['statusCode'] as int; String? actualBody; if (body is Map || body is List) { - actualBody = jsonDecode(body); + actualBody = jsonEncode(body); } else { actualBody = body.toString(); } - return MockResponse(actualBody!, headers, statusCode); + return MockResponse(actualBody, headers, statusCode); } } diff --git a/test/scenarios_test.dart b/test/scenarios_test.dart index fb453845..5d59b274 100644 --- a/test/scenarios_test.dart +++ b/test/scenarios_test.dart @@ -26,11 +26,12 @@ Future createGithubWithScenario(String scenario, throw Exception( 'Error loading scenario: $scenario\n${resp.statusCode}\n${resp.body}'); } - var j = json.decode(resp.body); + var j = json.decode(resp.body) as Map; return GitHub( - endpoint: j['url'], + endpoint: j['url'] as String, auth: const Authentication.withToken( - '0000000000000000000000000000000000000001')); + '0000000000000000000000000000000000000001'), + allowInsecureAuth: true); } /// Run scenario tests against ockokits fixtures-server diff --git a/test/unit/checks_test.dart b/test/unit/checks_test.dart index 2eb7e6c4..222a7683 100644 --- a/test/unit/checks_test.dart +++ b/test/unit/checks_test.dart @@ -98,7 +98,7 @@ const checkRunJson = '''{ }'''; const String expectedToString = - '{"name":"mighty_readme","id":4,"external_id":"","status":"completed","head_sha":"","check_suite":{"id":5},"details_url":"https://example.com","started_at":"2018-05-04T01:14:52.000Z","conclusion":"neutral"}'; + '{"name":"mighty_readme","id":4,"external_id":"","status":"completed","head_sha":"ce587453ced02b1526dfb4cb910479d431683101","check_suite":{"id":5},"details_url":"https://example.com","started_at":"2018-05-04T01:14:52.000Z","conclusion":"neutral"}'; const String newCheckRun = '{"name":"New CheckRun","id":12345,"external_id":"","status":"queued","head_sha":"","check_suite":{"id":123456},"details_url":"https://example.com","started_at":"2024-12-05T01:05:24.000Z","conclusion":"null"}'; @@ -106,7 +106,8 @@ const String newCheckRun = void main() { group('Check run', () { test('CheckRun fromJson', () { - final checkRun = CheckRun.fromJson(jsonDecode(checkRunJson)); + final checkRun = + CheckRun.fromJson(jsonDecode(checkRunJson) as Map); expect(checkRun.id, 4); expect(checkRun.name, 'mighty_readme'); @@ -114,7 +115,8 @@ void main() { }); test('CheckRun from freshly created and encoded', () { - final checkRun = CheckRun.fromJson(jsonDecode(newCheckRun)); + final checkRun = + CheckRun.fromJson(jsonDecode(newCheckRun) as Map); expect(checkRun.id, 12345); expect(checkRun.name, 'New CheckRun'); @@ -215,7 +217,8 @@ void main() { } ] }'''; - final checkRun = CheckRun.fromJson(jsonDecode(checkRunJson)); + final checkRun = + CheckRun.fromJson(jsonDecode(checkRunJson) as Map); expect(checkRun.id, 10); expect(checkRun.name, 'mighty_readme'); @@ -224,7 +227,8 @@ void main() { test('CheckRun toString', () { // indirectly tests the toJson method as well. - final checkRun = CheckRun.fromJson(jsonDecode(checkRunJson)); + final checkRun = + CheckRun.fromJson(jsonDecode(checkRunJson) as Map); expect(checkRun, isNotNull); final checkRunString = checkRun.toString(); expect(checkRunString, isNotNull); diff --git a/test/unit/checksuite_test.dart b/test/unit/checksuite_test.dart index d9571bc9..c3911e1c 100644 --- a/test/unit/checksuite_test.dart +++ b/test/unit/checksuite_test.dart @@ -45,7 +45,8 @@ const String expectedToString = void main() { group('Check suite', () { test('CheckSuite fromJson', () { - final checkSuite = CheckSuite.fromJson(jsonDecode(checkSuiteJson)); + final checkSuite = CheckSuite.fromJson( + jsonDecode(checkSuiteJson) as Map); expect(checkSuite.id, 5); expect(checkSuite.headBranch, 'main'); @@ -90,7 +91,8 @@ void main() { } ] }'''; - final checkSuite = CheckSuite.fromJson(jsonDecode(checkSuiteJson)); + final checkSuite = CheckSuite.fromJson( + jsonDecode(checkSuiteJson) as Map); expect(checkSuite.id, 10); expect(checkSuite.headBranch, 'master'); @@ -111,7 +113,8 @@ void main() { "conclusion": "skipped", "pull_requests": [] }'''; - final checkSuite = CheckSuite.fromJson(jsonDecode(checkSuiteJson)); + final checkSuite = CheckSuite.fromJson( + jsonDecode(checkSuiteJson) as Map); expect(checkSuite.id, 10); expect(checkSuite.headBranch, null); diff --git a/test/unit/common/model/misc_test.dart b/test/unit/common/model/misc_test.dart index a7cbf3c2..c529d5da 100644 --- a/test/unit/common/model/misc_test.dart +++ b/test/unit/common/model/misc_test.dart @@ -47,8 +47,8 @@ void main() { "used": 1 } }'''; - final rateLimit = - RateLimit.fromRateLimitResponse(jsonDecode(rateLimitJson)); + final rateLimit = RateLimit.fromRateLimitResponse( + jsonDecode(rateLimitJson) as Map); expect(rateLimit.limit, 5000); expect(rateLimit.remaining, 4999); diff --git a/test/unit/common/model/pulls_test.dart b/test/unit/common/model/pulls_test.dart index f4d5fe65..0a1fce44 100644 --- a/test/unit/common/model/pulls_test.dart +++ b/test/unit/common/model/pulls_test.dart @@ -238,7 +238,8 @@ const String samplePullRequest = ''' void main() { group('Pull Request fromJson', () { test('Node ID is collected', () { - final pullRequest = PullRequest.fromJson(jsonDecode(samplePullRequest)); + final pullRequest = PullRequest.fromJson( + jsonDecode(samplePullRequest) as Map); expect(pullRequest, isNotNull); expect(pullRequest.nodeId, "PR_kwDOA8VHis5QItg_"); }); diff --git a/test/unit/issues_test.dart b/test/unit/issues_test.dart index 512d83a3..9bc222ef 100644 --- a/test/unit/issues_test.dart +++ b/test/unit/issues_test.dart @@ -53,8 +53,8 @@ const String testIssueCommentJson = ''' void main() { group('Issue Comments', () { test('IssueComment from Json', () { - final issueComment = - IssueComment.fromJson(jsonDecode(testIssueCommentJson)); + final issueComment = IssueComment.fromJson( + jsonDecode(testIssueCommentJson) as Map); expect(1352355796, issueComment.id); expect('MEMBER', issueComment.authorAssociation); expect('CaseyHillers', issueComment.user!.login); diff --git a/tool/process_github_schema.dart b/tool/process_github_schema.dart index 14994289..c947571b 100644 --- a/tool/process_github_schema.dart +++ b/tool/process_github_schema.dart @@ -602,7 +602,8 @@ void main(List arguments) { 'Command must be run with one argument, the file name of the schema to process.'); exit(1); } - Object schema = json.decode(File(arguments.single).readAsStringSync()); + final schema = + json.decode(File(arguments.single).readAsStringSync()) as Object; assure(schema is Map, () => 'schema is not a JSON object'); var rootType = process(schema as Map); rootType.visit((GenType type) {