Skip to content

[generator] Warn if type name matches enclosing namespace name. #1010

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 2 commits into from
Jul 14, 2022

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jul 12, 2022

Fixes: #982

As explained in the Framework Design Guidelines:

❌ DO NOT use the same name for a namespace and a type in that namespace.

For example, do not use Debug as a namespace name and then also provide a class named Debug in the same namespace. Several compilers require such types to be fully qualified.

Since this is not a warning in Roslyn, and bindings code is autogenerated, it can be hard for bindings authors to notice that they are making this mistake. We can help surface this by emitting a warning if a type name matches its enclosing namespace.

This warning is BG8403.

Example:

warning BG8403: Type `Android.Graphics.Drawable.Drawable` has a type name which matches the enclosing namespace name.
See https://aka.ms/BG8403 for more information.

The provided link explains the issue and possible solutions.

Testing Note

It is very hard to unit test generator warning/error reporting, since Report is a global static class. Add a static property OutputDelegate that allows the tester to temporarily set a different output method that tests can use to verify Report output. These tests should be marked [NonParallelizable] since they affect the global state shared with any other running tests.

@jpobst jpobst force-pushed the namespace-warning branch 2 times, most recently from e6824c2 to 0898fab Compare July 12, 2022 19:34
@jpobst jpobst marked this pull request as ready for review July 12, 2022 19:48
@@ -7,6 +7,7 @@ namespace Java.Interop.Tools.Generator
public class Report
{
public static int? Verbosity { get; set; }
public static Action<string>? OutputDelegate { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency elsewhere, this should be an Action<TraceLevel, string>? See also ab3c2b2.

@jpobst jpobst force-pushed the namespace-warning branch from 0898fab to 146213f Compare July 14, 2022 18:26
@jonpryor jonpryor merged commit 45fe392 into main Jul 14, 2022
@jonpryor jonpryor deleted the namespace-warning branch July 14, 2022 19:42
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generator should warn about type names matching enclosing package names
2 participants