Skip to content
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

InternalsVisibleTo In Nonstandard Location #3029

Open
benrr101 opened this issue Nov 20, 2024 · 1 comment
Open

InternalsVisibleTo In Nonstandard Location #3029

benrr101 opened this issue Nov 20, 2024 · 1 comment
Labels
➕ Code Health Issues/PRs that are targeted to source code quality improvements. ✨ Good first issue Issues that are simple enough for newcomers

Comments

@benrr101
Copy link
Contributor

Description: In SqlConnection, there is an [assembly:InternalsVisibleTo...] attribute tucked at the beginning of the class. This is more/less in a non-standard location and makes was an unexpected surprise when I spotted it.

Fix: Move all InternalsVisibleTo attributes to AssemblyInfo.cs

@benrr101 benrr101 added ➕ Code Health Issues/PRs that are targeted to source code quality improvements. ✨ Good first issue Issues that are simple enough for newcomers labels Nov 20, 2024
@edwardneal
Copy link
Contributor

edwardneal commented Nov 23, 2024

I agree. The current InternalsVisibleTo attribute is:

[assembly: InternalsVisibleTo("System.Data.DataSetExtensions, PublicKey=" + Microsoft.Data.SqlClient.AssemblyRef.EcmaPublicKeyFull)]
// NOTE: The current Microsoft.VSDesigner editor attributes are implemented for System.Data.SqlClient, and are not publicly available.
// New attributes that are designed to work with Microsoft.Data.SqlClient and are publicly documented should be included in future.

Which internals does System.Data.DataSetExtensions need to use though? I read the comment as an indicator that the VS designer is currently linked against System.Data.SqlClient, so the InternalsVisibleTo attribute isn't in use. It might be worth reviewing the internal classes that System.Data.DataSetExtensions needs to reference and considering what public API surface could replace those references; without that, we can't make any variable naming changes to internal classes without risking breaking Visual Studio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Issues/PRs that are targeted to source code quality improvements. ✨ Good first issue Issues that are simple enough for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants