Compiler or analyzer should warn for lower-cased type names

[jcouv]

During LDM discussion on records, there was interest to have an analyzer warn on lower-cased type names. The motivation is that knowing that we discourage lower-cased type names would allow us to introduce new keywords (like record) with less concern about the breaking change.

I'm not sure whether this should be done by the compiler, a roslyn analyzer or some BCL analyzer.

Design:

• We use char.IsLower(...) for lower-case check.
• @ prefix avoids warning.
• All named types, included delegate types, are checked.
• Type and namespace aliases are checked too.

FYI @mikadumont @jmarolf

[CyrusNajmabadi]

If this is about the language and our ability to add such types in a more confident manner, then i think this should be part of the compiler proper. That way every gets it, and everyone sees that this is the official guidance from the team on what you should/shouldn't do.

[jcouv]

Just realized, one issue we'll have to figure out is how we define "lower-case".

[CyrusNajmabadi]

All(char.IsLower)?

[jmarolf]

Agree with @CyrusNajmabadi that the ideal here is for the compiler to emit a new warning as part of warning wave 7. If that cannot happen then we can consider shipping it as an analyzer. Considering that this is a fundamental part of C# it would seem to be appropriate for the dotnet/roslyn repo since dotnet/roslyn-analyzers is for warnings about .NET libraries instead of core language concepts.

[jcouv]

@jaredpar Should we contemplate doing this as part of warning wave 6 (ie. in .NET 6)?

[Youssef1313]

Will prefixing with @ remove the warning?

Otherwise, I can't think of a good workaround, for example, for Xamarin's System.nuint:

[jcouv]

From discussion with Jared, it's too late for .NET 6. We could make this any early preview for release after that (such as 17.1).

@Youssef1313 That's an excellent suggestion. It would make sense given that the @ prefix allows you to use other keywords as identifiers.

[JoeRobich]

Design Meeting Notes:

• Can this be covered by the NamingStyleAnalyzer?
  • By default these diagnostics are suggestions and compiler would prefer warnings.
  • Naming styles are configurable which is at odds with what the compiler is asking for.

Proposal: Keep IDE analyzers as they are. The compiler will add a diagnostic behind a warning wave for VS 17.1 Preview 1.

[jcouv]

FYI, the proposal was discussed and refined in LDM 10/13/2021. Notes upcoming

[333fred]

Notes: https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-10-13.md#warning-on-lowercase-type-names

[xoofx]

Hello there,

In Unity, we are using lower-case names for some special types that we consider as "primitives" (e.g float4)

So we have 2 questions:

• Will it be possible to disable this warning project wide?
• Does it apply only to declarations or If another project is consuming these names (e.g method with a parameter having these types), it will propagate to users projects?

[CyrusNajmabadi]

Will it be possible to disable this warning project wide?

Yes, as a warning, you can generally disable it for your entire compile.

it will propagate to users projects?

Yes, it would likely apply downstream.

(e.g float4)

This would likely not be an issue. We would be looking for 'all lower cast ascii identifiers'. e.g. things like var or dynamic or async. Something like float4 contains a number and would not be affected.

[xoofx]

Thanks for the response @CyrusNajmabadi, sorry I cross posted (thought I deleted this one above)

[soc]

@CyrusNajmabadi https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/built-in-types

[Youssef1313]

@soc Both alias C# keywords (e.g. string) and the .NET runtime types (e.g. System.String) works. Nothing has changed here. This is about user-defined types that are named in all lowercase. For example:

class abc { } // warning

[soc]

@Youssef1313 The point is consistency: Do not make up rules for language users that you, as a language developer, don't intend to follow.

[Youssef1313]

I'm not really sure what you mean.

[davidwengier]

I think they mean that if users aren't allowed to define a type that is all lowercase, then the runtime/language shouldn't either.

[Youssef1313]

If that's what @soc meant, then there are two things:

1. string, int, etc, are language keywords that map to PascalCase types. So they are not lowercase types.
2. Even if the runtime (or a regular user) wanted to define a lowercase type, you can always prefix it with @. For example, I'd imagine Xamarin will do that if they encountered the warning for their nint (and nuint).

[CyrusNajmabadi]

@Youssef1313 The point is consistency: Do not make up rules for language users that you, as a language developer, don't intend to follow.

The point is exactly to have that block. We want the language to be able to use these keywords without colliding with anything a user does. If we continue to use the same space of names (like today), then nothing will change and we will have the same potential collision problem.

[soc]

@Youssef1313

1. As users cannot define their own "type keywords", the distinction is completely irrelevant from a user perspective.
2. I don't.

[CyrusNajmabadi]

@soc

As users cannot define their own "type keywords", the distinction is completely irrelevant from a user perspective.

Users can define types that could conflict with keywords we want to use in the language in the future. We want to explicitly steer people away from that. The entire point is to bifurcate the space of identifiers to ones we can safely use in the language without worry about conflit, and those that anyone can use themselves in their code. The point is to have a split.

[soc]

@CyrusNajmabadi Agreed. Not sure what you are arguing against?

[CyrusNajmabadi]

@CyrusNajmabadi Agreed. Not sure what you are arguing against?

You said to Youssef1313:

@Youssef1313 The point is consistency: Do not make up rules for language users that you, as a language developer, don't intend to follow.

I am arguing against that idea.

The point is exactly to make up rules for users that the language itself won't follow. We are trying to split this space entirely so that we can have more names like int and not have any chance of them colliding with users.

[soc]

Perhaps this issue and the discussion notes are misleading, because it's simply not "warn for lower-cased type names" then.

[Youssef1313]

It is warn for lower-cased type names. So class abc { } will warn (it's gated under warning level 7 I believe?)

[CyrusNajmabadi]

because it's simply not "warn for lower-cased type names" then.

> During LDM discussion on records, there was interest to have an analyzer warn on lower-cased type names. 

And

#56905
Report a level 7 warning for lower-cased type names

It is exactly that. :)

[soc]

So when compiling the BCL we'll get warnings for int, float, string?

[CyrusNajmabadi]

So when compiling the BCL we'll get warnings for int, float, string?

No. The bcl does not define those. So there's nothing to warn on.

[333fred]

I think there might be a bit of confusion: int, float, string, and all the rest of the keywords on https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/built-in-types are not type names. They are C# keywords. The actual name for string is System.String, which does indeed follow this convention.

[soc]

@CyrusNajmabadi Then "warn for lower-cased type names, except our own" or "warn for lower-cased type names, except for type keywords" would be correct, but the title as given is misleading.

[soc]

@333fred I'm not confused thank you.

[Youssef1313]

There is really no exception. int (and friends) are keywords. They're not type names (the corresponding type names are System.Int32 and friends - as @333fred pointed out). The compiler warns for lower-cased type names.

[soc]

@Youssef1313 If you discuss type names then the "type-ness" of "type keywords" is more relevant that their "keyword-ness".

[CyrusNajmabadi]

@CyrusNajmabadi Then "warn for lower-cased type names, except our own"

Those are not type-names. They are built in keywords. We're not warning on keywords here, only type names. Thanks :)

but the title as given is misleading.

The title is accurate (as is the linked issue). You're welcome to ask for clarity if the terminology isn't something you're totally familiar with of course.

In this case though your point you raised isn't valid: "Do not make up rules for language users that you, as a language developer, don't intend to follow."

We do you define type names in the language. That comes externally. We do define keywords, and we want to increase the likelihood that our keywords will not collide with user type names.

@Youssef1313 If you discuss type names then the "type-ness" of "type keywords" is more relevant that their "keyword-ness".

I'm not sure what point you're trying to make here. We're warning on lowercase type names because that's the space that we want to reserve for us for keywords (type or otherwise).

[CyrusNajmabadi]

@soc the relevance is the 'keyword-ness'. We want to be able to make keywords that do not collide with user types. It has nothing to do with the keyword being type-like (like int). For example, even basic non-type keywords (like record, which doesn't correspond to any .net type**) had a problem because it might collide with a user type. I think you're not understanding the reason for this work, or how it relates to the language so far, or what we're considering in the future.

** Other examples are things like notnull, or unmanaged, again, not aliases for some .Net type. They're just things we want as identifiers we can use as keywords safely, but which we have to expend extra implementation energy on as someone may have themselves defined a type with this name.

[davidwengier]

Is there already a recommended time table on which users can start replacing int, float, string and friends?

If you or anyone else wants to disallow using language keywords like these in your own projects, you can enable this style rule and make it a warning or error, as you see fit: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0049