All types should be partial by default #3748
Replies: 33 comments 4 replies
-
I'm somewhat interested in this idea, though I will also want to see what the initial reaction to source generators is as well. I certainly wouldn't want all types to be automatically partial. I'm more amenable to the "all types can can have at most one file not marked partial". My ideal here would be "if there are multiple user-defined files for a single type, that type must be marked partial in every definition." However, that would require us to define what a generated file vs a user-defined file is. |
Beta Was this translation helpful? Give feedback.
-
I like the certainty of knowing that a non-partial type is not going to be modified by a source generator: I like having a marker that sticks out saying "this class is more than it seems: go looking if you want to see what else it can do". Even if you don't happen to know that a particular attribute triggers a source generator, partial tells you unambiguously that there's more going on somewhere else. Source generators are effectively hidden magic. That can be a good thing, but it's nice to know clearly where magic is happening and where it definitely isn't. |
Beta Was this translation helpful? Give feedback.
-
I wasn't sure I liked this idea, but my next thought was http://gafter.blogspot.com/2017/06/making-new-language-features-stand-out.html. And |
Beta Was this translation helpful? Give feedback.
-
Yeah it doesn't warn you specifically about source generators, but it says that something exists somewhere else, and you can ask VS to take you there. One of those places might be a source generated file, or it might not. But it's an indicator to go and look. I appreciate gafter's point, but your proposal lets source generators change things without any indication whatsoever. I don't think an indication needs to stand out necessarily, but I think there should be something. |
Beta Was this translation helpful? Give feedback.
-
In theory the editor should be able to indicate to the programmer that a class is affected by a source generator, right? I feel like that's better than requiring people to sprinkle |
Beta Was this translation helpful? Give feedback.
-
Once source generators are more common, then it will just be default knowledge that source generators can add members to objects. You wont need a visual cue that that can happen, because it will be as obvious you as the fact that other files can declare other types, even if they're not declared in this file. I think the requirement to use |
Beta Was this translation helpful? Give feedback.
-
I have the complete opposite view to |
Beta Was this translation helpful? Give feedback.
-
To reduce this to a single question: Should source generators be able to be used to add DebuggerDisplay to every single type in my project? If yes, then the only way to enable this practically is to remove the need for the partial keyword. Otherwise every single type will have to be declared partial, which is extremely noisy, ruins the git blame, and at which point the partial keyword tell me nothing useful. If no, then requiring the partial keyword is a reasonable way to prevent this happening. |
Beta Was this translation helpful? Give feedback.
-
I'm also concerned about the viability of source generators that are intended to affect large amounts of the codebase. In my opinion, instead of potentially damaging the value of I don't think |
Beta Was this translation helpful? Give feedback.
-
That's a neat single question that shows our differences. To me, the answer is "absolutely not". The idea of a source generator changing every type in my project is abhorrent to me. But clearly you see it as a positive. |
Beta Was this translation helpful? Give feedback.
-
While it wasn't merged, judging by Unity-Technologies/roslyn#17 it appears that this has caused the Unity developers some headaches. |
Beta Was this translation helpful? Give feedback.
-
Right, but I'd rather not have to individually use "Go To Definition" on every single type to figure out whether the source generator has added any members. I would certainly not expect a source generator to be modifying every single type in my assembly. Adding And if you do want to be able to modify every type in an assembly, then change your VS template to add People just don't like impenetrable magic. They don't like things happening that are hard to discover. It's one of the reasons so many people hate things like ioc containers. I do like the idea of the IDE showing an indication that a type has source-generated members. Possibly even going so far as to show the signatures of these generated members in the user's source file. But I do think that only allowing source generators to touch types which have been explicitly whitelisted by the user can only be a good thing. It also gives users an easy way of stopping a source generator from modifying a type if it's misbehaving in some case. |
Beta Was this translation helpful? Give feedback.
-
This will probably not work well. My generator for example will always generate the type, and then expects the user to fix the error if it's not partial. Even those that do special case this will probably produce a diagnostic to warn that it's missing a partial. Source generators who are going to the effort to support this should instead just support an explicit suppression attribute. That will work much better, and allow blocking one source generator and not another. |
Beta Was this translation helpful? Give feedback.
-
It's not about any sort of indication. It's about me not wanting generators to be able to touch my types unless i give them explicit permission with 'partial'. |
Beta Was this translation helpful? Give feedback.
-
You might want to fix that 🙂 Diagnostics are suppressible, that's OK. |
Beta Was this translation helpful? Give feedback.
-
@IllidanS4 that would take the language down the path of multiple dialects of C#, something the LDM has been extremely clear about avoiding. |
Beta Was this translation helpful? Give feedback.
-
@theunrepentantgeek Will it really? You have Tell me, what does this class do? class C
{
} If you answered surely nothing, you would not be correct. It derives from Pragmatically, this cannot work without any sort of explicit configuration, or things will take a very confusing path very quickly. |
Beta Was this translation helpful? Give feedback.
-
I'd certainly be correct with valid C# code today because if there was Introducing a per-project/per-file toggle makes it ambiguous, which was @theunrepentantgeek's point. |
Beta Was this translation helpful? Give feedback.
-
@PathogenDavid I was speaking in the context of this propsal of course. Introducing a toggle does make it ambiguous, but having the current behaviour as the default one will force everyone to either state it is toggled or assume it is not otherwise. |
Beta Was this translation helpful? Give feedback.
-
@IllidanS4 Yes it will, really. "Dialect" is the term the language design team uses. They regret the /unsafe and /checked switches and they added the /nullable switch only because the value of NRTs was so huge and there was no good way around it. |
Beta Was this translation helpful? Give feedback.
-
Is DebuggerDisplayAttribute really even all that useful anymore? Since Visual Studio added pinnable properties while debugging, ToString and DebuggerDisplay have become far less useful to me. |
Beta Was this translation helpful? Give feedback.
-
@MgSam Yes. It saves first-time discovery about what properties are interesting and it saves scrolling, waiting, and clicking. All things which are lost if you |
Beta Was this translation helpful? Give feedback.
-
The "interesting" properties are type-specific, surely? |
Beta Was this translation helpful? Give feedback.
-
This would make it more difficult to verify that Platform Invoke struct types have the same fields as in the unmanaged API. |
Beta Was this translation helpful? Give feedback.
-
#950 has some prior discussion. I closed it eventually. |
Beta Was this translation helpful? Give feedback.
-
Could a project property that tells the compiler that every class under specified namespaces get "partial" do this? |
Beta Was this translation helpful? Give feedback.
-
What about adding another keyword to the second and subsequent class Foo
{
}
// elsewhere - say, generated code
explicit partial class Foo
{
}
// no errors when "explicit" is added |
Beta Was this translation helpful? Give feedback.
-
Allowing something else to opt my own types into being partial seems go against one of the major design reasons behind partial in the first place. Namely that the type author be in control here and that the type be apparent at its declaration that it was partial. |
Beta Was this translation helpful? Give feedback.
-
Is using This would allow the unhindered pathway to ubiquitousness for source generators while still preventing user born issues with an implicit partial. As for @CyrusNajmabadi's argument regarding opt-in for Source Generators on types. That would be better handled in the source generators rather than at the language level. If a source generator is touching types/methods/properties you don't want, either enhance the source generator to add better granularity of control or stop using it. |
Beta Was this translation helpful? Give feedback.
-
They're opt-in to have a source generator pick up something as a basis for generation. That generation doesn't necessarily have to be a partial that gets merged into the original. It could be other things - subclasses, utility classes, extension methods, the list goes on. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Most source generators work by adding members to user declared partial types. This means that the user needs to declare the type with the
partial
modifier. This adds a lot of noise to code bases which make heavy use of code generators.I very much hope that code generators will become much more commonplace post c# 9.0. For example I expect it will be common for code bases where performance and security are low prioirities but debuggability is paramount to use source generators to automatically implement ToString/DebuggerDisplay on all types. Other code bases may use source generators to override the default record behaviour for example.
However with the need to explicitly declare every single type
partial
I can't see this happening.I propose that all types become partial by default to allow source generator usage to be more implicit and less noisy.
Objection 1: what if I accidentally declare two types with the same name.
A simple solution would be to only allow at most one declaration of a type to omit the
partial
modifier. All others must explicitly declare the type as partial. This will allow users to omitpartial
whilst source generators will have to add it.Objection 2: I like the explicitness of the
partial
keyword.This is a valid point, but I have a number of counterpoints:
partial
keyword doesn't do a great job at this anyway. It's used for a number of other purposes, not just to tell you that some code has been generated. If the language team feels like source generators need some special syntax, then they should add that, not usepartial
as something that happens to sorta work.partial
is superfluous. And those which should apply to every type. For thesepartial
is unwanted and way too noisy.Beta Was this translation helpful? Give feedback.
All reactions