Skip to content

chore: modernize EnvironmentVariableService#1788

Merged
JamieMagee merged 1 commit intomainfrom
cleanup/env-var-service
Apr 27, 2026
Merged

chore: modernize EnvironmentVariableService#1788
JamieMagee merged 1 commit intomainfrom
cleanup/env-var-service

Conversation

@JamieMagee
Copy link
Copy Markdown
Member

What

Cleans up EnvironmentVariableService.GetEnvironmentVariable:

  • Replaces string.Compare(x, name, true) == 0 (a .NET Framework idiom) with string.Equals(x, name, StringComparison.OrdinalIgnoreCase).
  • Short-circuits to Environment.GetEnvironmentVariable(name) on Windows, where the lookup is already case-insensitive — no need to enumerate every environment variable.
  • Drops #nullable disable from the file.

Why

The original implementation predates net8.0 and always paid the cost of enumerating every environment variable, even on Windows where the platform handles case-insensitivity natively.

Part 4 of a 6-PR cleanup series removing .NET-Framework-era anachronisms.

@JamieMagee JamieMagee requested a review from a team as a code owner April 17, 2026 18:23
@JamieMagee JamieMagee requested review from Copilot and ryanbrandenburg and removed request for Copilot April 17, 2026 18:23
@JamieMagee JamieMagee force-pushed the cleanup/env-var-service branch from 91d78bd to ec11315 Compare April 17, 2026 21:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Modernizes EnvironmentVariableService.GetEnvironmentVariable to better align with net8.0 and platform behavior, reducing unnecessary work on Windows while keeping cross-platform case-insensitive lookup behavior.

Changes:

  • Updated IEnvironmentVariableService.GetEnvironmentVariable to return string? to reflect the fact it can return null.
  • Optimized Windows behavior by short-circuiting to Environment.GetEnvironmentVariable(name) (no enumeration).
  • Replaced string.Compare(..., true) with string.Equals(..., StringComparison.OrdinalIgnoreCase) and removed #nullable disable from the implementation file.
Show a summary per file
File Description
src/Microsoft.ComponentDetection.Contracts/IEnvironmentVariableService.cs Updates interface return nullability for GetEnvironmentVariable.
src/Microsoft.ComponentDetection.Common/EnvironmentVariableService.cs Optimizes Windows lookup and modernizes case-insensitive comparison; enables nullable context.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Replace string.Compare(.., true) with StringComparison.OrdinalIgnoreCase. Short-circuit to Environment.GetEnvironmentVariable on Windows, where it is already case-insensitive. Drop #nullable disable on the file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JamieMagee JamieMagee force-pushed the cleanup/env-var-service branch from ec11315 to 0138013 Compare April 27, 2026 18:00
@github-actions
Copy link
Copy Markdown

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

  • The detector detects more or fewer components than before
  • The detector generates different parent/child graph relationships than before
  • The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

@JamieMagee JamieMagee enabled auto-merge (squash) April 27, 2026 22:54
@JamieMagee JamieMagee merged commit ed70066 into main Apr 27, 2026
15 checks passed
@JamieMagee JamieMagee deleted the cleanup/env-var-service branch April 27, 2026 22:54
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.

3 participants