Powershell: Add-Type now quietly ignores attempts to change a previously added .NET type's definition in the same session

[mklement0]

This is a regression from Windows PowerShell, where the - useful - behavior is as follows:

• If you repeatedly call Add-Type -TypeDefinition/-MemberDefinition for the same target type, only the _first_ invocation adds the type to the session and subsequent calls are benign - and fast - no-ops, _if_ the source code is unchanged.

• If you modify the source code, you get an error along the lines of:
  Add-Type : Cannot add type. The type name '<name>' already exists.

This behavior is sensible, as it alerts you to the fact that trying to change the type in-session isn't possible.

By contrast, PowerShell Core now quietly _ignores_ attempts to change the type, which means that you may not notice that your changes didn't take effect.

Steps to reproduce

Run the following Pester test

{
  Add-Type -TypeDefinition @'
  public class Foo {}
'@

  # Try to redefine the type with different source code.
  Add-Type -TypeDefinition @'
  public class Foo { public int Bar {get {return 42;} } }
'@

} | Should -Throw

Expected behavior

The test should succeed, because an error should be emitted.

Actual behavior

The test fails, because no error is emitted, because the second Add-Type is effectively quietly ignored [a _new_ assembly is actually being created - see
[@daxian-dbw's comment below](https://github.com/PowerShell/PowerShell/issues/9575#issuecomment-491634962)]

Environment data

PowerShell Core v6.2.0

[iSazonov]

Add-Type was fully re-implemented on Roslyn (CodeDom was used in Windows PowerShell).
And now we actively use caching - if we can find a type we use them and don't compile again. If I remember right this was accepted as preferred behavior.

[mklement0]

@iSazonov:

Windows PowerShell is capable of detecting if the source code changed and only if it did does it complain - helpfully so; otherwise, the call is a quiet - and fast - no-op.

Are you saying the same can't be done in PowerShell Core?

I don't think anyone prefers the behavior of having their attempt to define a type quietly ignored.

[iSazonov]

Windows PowerShell is capable of detecting if the source code changed

There was very simple caching.
Current cache is very effective in most use cases. For example, we do not re-read the file every time, but just see if timestamp was changed. We don't cache based on source text but on SourceTree.

Are you saying the same can't be done in PowerShell Core?

No. Our thoughts was that the behavior is more safe in multi runspace environment. For example, we could destroy foregroud job by running a script which re-define a custom type. Recommendation for script/module authors is to define types in custom namespace.

quietly ignored.

There should be a verbose message.

[mklement0]

There should be a verbose message.

That's what this issue is asking for.

Note that it's not just about clashes between _different_ authors. It's also about debugging by a single author, rerunning a script in a given session after having modified the source code, not realizing that the redefinition won't be possible in the same session - from what I understand, it's technically impossible to redefine a given .NET type in a session, because unloading assemblies isn't possible, right?

So we're in agreement, then? Your original response sounded like you favored the quiet ignoring.

[daxian-dbw]

If you modify the source code, you get an error along the lines of:
Add-Type : Cannot add type. The type name '' already exists.

This should be the expected behavior. We should fix Add-Type in PS core.

subsequent calls are benign - and fast - no-ops, if the source code is unchanged.

Agreed that a verbose message should be added in this case.

[mklement0]

Thanks, @daxian-dbw.

A message to the verbose stream _is_ already being issued (but only in PS _Core_); i.e., you need to use -Verbose to see it: The source code was already compiled and loaded.

I think that's sufficient - or were you thinking the message should be displayed _unconditionally_?

I think it's better to be silent by default; this allows users not to have write additional logic around Add-Type calls: simply call them every time in your script, relying on subsequent calls to be quiet no-ops.

[fMichaleczek]

I already remove additional logic around Add-Type calls and i don't think I'm the only one.

[iSazonov]

Note that it's not just about clashes between different authors. It's also about debugging by a single author, rerunning a script in a given session after having modified the source code, not realizing that the redefinition won't be possible in the same session - from what I understand, it's technically impossible to redefine a given .NET type in a session, because unloading assemblies isn't possible, right?

1. Current behavior is nice for regular use.
2. For development author have to re-run pwsh. I do so and don't think that it is a big problem.
3. Add-Type compiles to new assembly and the new assembly is used then - so we could "re-define" types (in script at least). This can be useful in dev but dangerous in all other scenarios. If I wrong @daxian-dbw will correct me.

[mklement0]

1. For development author have to re-run pwsh. I do so and don't think that it is a big problem.

I too don't think that the need to re-start the session is a problem, and I'm not asking to change that.

I do, however, think that _not alerting the user to the need_ for starting a new session is the problem, and it sounds @daxian-dbw agrees with that.

1. Add-Type compiles to new assembly and the new assembly is used then - so we could "re-define" types (in script at least). This can be useful in dev but dangerous in all other scenarios. If I wrong @daxian-dbw will correct me.

It isn't dangerous. If the source code is effectively the same - and that would be the _only_ scenario in which the quiet no-op (no error) occurs - then there's no problem, regardless of who calls Add-Type and how often.

[iSazonov]

If the source code is effectively the same

I mean that only full type name is the same - that is dangerous.

[mklement0]

I mean that only full type name is the same - that is dangerous.

That's not what we're talking about here.
We're talking about the case _where the source code hasn't changed_ for a given type.

Incidentally, the verbatim source-code caching that you believe is exclusive to Windows PowerShell seems to be in PowerShell Core as well, which I infer from the following:

The current PS Core code too knows the difference between the two scenarios (source code changed vs. unchanged), because the aforementioned verbose message is _only_ emitted in the _unchanged_ case; the changed case is completely silent.

Thus, it sounds like making the changed case emit a statement-terminating as in Windows PowerShell is all that is needed.

As an aside: I think that _verbatim_ source-code caching as the basis for determining whether a type's definition has changed is sufficient, not least because it allows for much faster testing than having to compile every time. As stated, verbatim checking seems to be in the code already.

[daxian-dbw]

A message to the verbose stream is already being issued (but only in PS Core); i.e., you need to use -Verbose to see it

@mklement0 I didn't know the verbose message was already there. I agree that's sufficient.

I'm not sure if we are all clear on the current behavior as demonstrated in the repro steps:

Add-Type -TypeDefinition @'
  public class Foo {}
'@

# Try to redefine the type with different source code.
Add-Type -TypeDefinition @'
  public class Foo { public int Bar {get {return 42;} } }
'@

Today, the second Add-Type actually generates another assembly with the type Foo that contains a Bar property. However, the use of [Foo] subsequently will have undefined behavior -- it could be resolved to either the first [Foo] that has an empty body, or the second [Foo] with a Bar property, because the type resolution will stop after finding the first Foo from all loaded assemblies. See the example below:

> $as =[System.AppDomain]::CurrentDomain.GetAssemblies() | ? -not Location | select -Last 2
> $as.FullName
0pyel1t2.h2v, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null
bycuxatk.jhq, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null

> $as[0].GetTypes()

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     False    Foo                                      System.Object

> $as[1].GetTypes()

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     False    Foo                                      System.Object

@iSazonov I don't think this is the right behavior for Add-Type, because there is no way for the user to reliably use the generated type. I think we should fix it to throw error when redefining the same type with different source code.

[mklement0]

Thanks, @daxian-dbw; it sounds like we're in complete agreement (despite the fact that I hadn't realized that another assembly was actually being created, but that point is moot, given the suggested resolution).

[iSazonov]

I don't think this is the right behavior for Add-Type, because there is no way for the user to reliably use the generated type. I think we should fix it to throw error when redefining the same type with different source code.

Above I just discribe this and I was sure that this was already done as we already have CheckDuplicateTypes() and AllNamedTypeSymbolsVisitor

$a = Add-Type -TypeDefinition @'
   public class Foo {}
 '@ -PassThru

$a1 = Add-Type -TypeDefinition @'
   public class Foo {}
 '@ -PassThru

$a.Assembly.FullName
bwb1lxup.0m4, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null

$a1.Assembly.FullName
bwb1lxup.0m4, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null

[mklement0]

I may have confused the issue by initially not realizing that a _new_ assembly is created in the case of changed source code (resulting in a type with the same full name that isn't reliably used when referenced by that name).

So, are we all on the same page now, @iSazonov?

To recap, the desired behavior is:

• If the source code didn't change, default to a fast and quiet no-op, although -Verbose allows emitting a message indicating that the existing definition is used - this part is already working.

• If the source code did change, do not generate a new assembly and instead throw a statement-terminating error indicating that the type cannot be changed in-session - this is how it works in Windows PowerShell and how it should work in PowerShell Core as well.

[iSazonov]

Yes, code examples clarified and I prepared the fix.

[msftbot[bot]]

:tada:This issue was addressed in #9609, which has now been successfully released as v7.0.0-preview.4.:tada:

Handy links:

• Release Notes