8000 Using Directive Placement by plioi · Pull Request #312 · fixie/fixie · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Using Directive Placement #312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Dec 9, 2023
Merged

Using Directive Placement #312

merged 13 commits into from
Dec 9, 2023

Conversation

plioi
Copy link
Contributor
@plioi plioi commented Dec 9, 2023

This repo has been placing using directives within namespace blocks. The apparent benefit was occasional brevity for "nearby" namespaces: within the Fixie.Tests namespace, you could simply say using Assertions to bring in the nearby namespace Fixie.Tests.Assertions. This was the only benefit, and switching to it was a Pyrrhic victory for two reasons:

  1. It does not play nicely with the more modern "global using" feature (explicit or implicit), forcing redundant directives and therefore less brevity.
  2. Switching away from this meager brevity unskillfully is risky as the meaning of some type name in a code file might silently change if you merely relocate the namespace directive below existing using directives. Fortunately most such conflicts would coincidentally surface as a compiler error based on usages of the identifier, but still: here be dragons.

This reorders using vs namespace as a step towards cleanly applying implicit/global using directives in a subsequent PR. By applying this code change separately from the implicit/global using directives, each can be more easily reviewed in isolation.

This was done manually. For each visited file, already-abbreviated using directives were first expanded to their fully-qualified equivalent, letting the IDE first confirm what the implied full name was. Once all using directives in a code file were fully-qualified, they could be safely cut and pasted before the namespace with confidence that the move could not alter the meaning of the following code.

Two name conflicts were encountered and resolved:

  1. Console tends to be ambiguous in the tests when we can alternately mean System.Console or Fixie.Console. We resolve this by always referring to System.Console... explicitly in the tests, and can mean Fixie.Console implicitly otherwise.

  2. TestResult can mean either Microsoft.VisualStudio.TestPlatform.ObjectModel.TestResult or Fixie.TestResult. We resolve this in both VsExecutionRecorder and VsExecutionRecorderTests by including the following alias after the namespace declaration:

    using TestResult = Microsoft.VisualStudio.TestPlatform.ObjectModel.TestResult;

@plioi plioi marked this pull request as ready for review December 9, 2023 20:05
@plioi plioi merged commit 2ec7a19 into main Dec 9, 2023
@plioi plioi deleted the using-directive-placement branch December 9, 2023 20:06
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
0