Skip to content

feat(AC0035): report Rest Client variables not initialized with a custom Http Client Handler - #571

Open
Arthurvdv wants to merge 4 commits into
mainfrom
feat/ac0035-rest-client-initialize-with-http-client-handler
Open

Arthurvdv wants to merge 4 commits into
mainfrom
feat/ac0035-rest-client-initialize-with-http-client-handler

Conversation

@Arthurvdv

@Arthurvdv Arthurvdv commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Summary

New ApplicationCop rule AC0035 RestClientInitializeWithHttpClientHandler, implementing Rule 2 of discussion #405 (the per-variable check). AC0033 (#568) reports once per object when the app implements no "Http Client Handler" at all; AC0035 reports each Codeunit "Rest Client" variable that does not receive one. Both fire independently.

ID Rule Reports when
AC0035 RestClientInitializeWithHttpClientHandler A local (any method or trigger body) or global variable of Codeunit "Rest Client" (2350) is initialized with Initialize(), Initialize(HttpAuthentication), Create(), Create(HttpAuthentication) or with the System Application's default codeunit 2360 "Http Client Handler", or is used without any initialization. Rest Client Impl. auto-initializes with the default handler on first use, so all of these send the request as the System Application (permission scope, outgoing web-service telemetry, no test mocking).

Design

  • Roots are locals and globals only. Parameters and return values are never roots: the caller owns initialization, so a helper that receives a var RestClient parameter is silent on its own.
  • Good initialization is any Initialize/Create whose handler-typed argument (bound to an Interface "Http Client Handler" parameter) has a static type other than codeunit 2360 (matched on id and name). Interface-typed variables, factory return values, parameters and handler codeunits from any app all pass, so the library pattern (main app passes its handler into a shared library that calls the Rest Client) is not reported.
  • Reach is the same object: a root passed to a procedure declared inside the same object syntax is followed into the callee (both var and by-value, since codeunit variables are reference types), unlimited depth, cycle-safe.
  • Uncertainty means silence. The root is never reported once it is passed to another object, a dependency, an interface method, an event or a built-in such as Clear, assigned from anything but its own Create, assigned to another variable, returned via exit, or an invocation on it fails to bind.
  • No ordering analysis and no request-method list: any member call other than Initialize/Create counts as use, so future Rest Client members need no uptake.
  • Location: the first default Initialize/Create call found directly on the root (own body for a local, any body of the object for a global); otherwise the variable name. A default call is reported even when a good call exists elsewhere (Initialize() re-initializes with the default handler).
  • Per-object RegisterSymbolAction on the same nine kinds as AC0033; root discovery on symbols only (LocalVariables, GetMembers()), one GetSemanticModel per object with a root, binding only the bodies that own a local root or spell a global root's name. Self-contained per object, so partial-analysis passes are safe.
  • Warning, enabled by default, category Design, no settings, no CodeFix, no version gate (every SDK member used exists at ns2.0 / AL 12).

Deliberate non-reports

Parameters and return values, Clear(RestClient), anything passed out of the object, Codeunit::"Rest Client" object references, arrays/lists/interface-typed variables, declared-but-unused variables, test and test-runner codeunits, obsolete objects and obsolete procedures.

Verification

  • TDD: the first commit (wiring, stub analyzer, 45 fixtures) ran red on the filtered test: Failed: 18, Passed: 27 (all 18 HasDiagnostic cases failing, all 27 NoDiagnostic passing); after the implementation commit Passed: 45 on net10.0 and on the net8.0 leg (GITHUB_ACTIONS=true … -p:NavTargetFramework=net8.0).
  • dotnet build ALCops.sln clean; ApplicationCop and Common Release builds with ContinuousIntegrationBuild=true (netstandard2.1 / net8.0 / net10.0) clean; dotnet format ALCops.sln --verify-no-changes clean; full dotnet test ALCops.sln green; Validate-Rules.ps1 OK.
  • An ad-hoc NoException pass over all 45 fixtures (markers stripped) showed no swallowed analyzer exceptions, and every fixture reports exactly the expected AC0035 count on the expected span.
  • Code review (Sonnet, effort high) run before opening this PR: no correctness finding survived verification; its two low findings (an unbound call could pair the root with the wrong callee parameter; repeated body serialization in the global pre-filter) are addressed in the last commit, and the deliberate DescendKinds duplication with AC0033 is recorded in the rule doc.

Docs: companion page in ALCops/alcops.dev#194.

🤖 Generated with Claude Code

Arthurvdv and others added 3 commits September 27, 2026 19:56
…itializeWithHttpClientHandler

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tom Http Client Handler

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he global pre-filter

A call that fails to bind may pair the root with the wrong parameter, so following it
could track a symbol that never appears in the callee; the root is now silenced instead.
The body text used by the global pre-filter is computed once per method rather than once
per global and method pair. The rule doc records the deliberate DescendKinds duplication.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Arthurvdv

Copy link
Copy Markdown
Member Author

CI note: the red "Test results" check is a flake, not a regression. On attempt 1 the timing assertion in ALCopsSettingsRemoteRecoveryTests.CancelledCompilation_ClosesHttpRequestPromptly_AndNextCompilationRecovers failed on the 12.1.13.35966 leg only (Common, untouched by this PR); every AC0035 test passed on all 35 legs. The leg was rerun and the test passed (Passed CancelledCompilation_ClosesHttpRequestPromptly_AndNextCompilationRecovers [10 ms], job 108681078778), but the Test Report merges the artifacts of both attempts, so the attempt-1 failure is still counted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant