Fix dynamic class assembly name - #5292
Conversation
Using the assembly name to hint at the source of the classes was
problematic in multiple ways.
This change stores the actual filename in an attribute on the assembly.
So for a given type, one can get the assembly this way:
[SomeType].Assembly.GetCustomAttributes() |
? { $_ -is [System.Management.Automation.DynamicClassImplementationAssemblyAttribute] } |
% { $_.ScriptFile }
6a668b3 to
7ca8ac8
Compare
| typeof(DynamicClassImplementationAssemblyAttribute).GetProperty(nameof(DynamicClassImplementationAssemblyAttribute.ScriptFile)) }; | ||
| var propertyArgs = new object[] { scriptFile }; | ||
| var fieldInfoList = Utils.EmptyArray<FieldInfo>(); | ||
| var fieldArgs = Utils.EmptyArray<object>(); |
There was a problem hiding this comment.
fieldInfoList and fieldArgs are not used anywhere.
There was a problem hiding this comment.
Thanks, cut and paste I guess.
| { | ||
| yield return new CustomAttributeBuilder(typeof(DynamicClassImplementationAssemblyAttribute).GetConstructor(Type.EmptyTypes), s_emptyArgArray); | ||
| var ctor = typeof(DynamicClassImplementationAssemblyAttribute).GetConstructor(Type.EmptyTypes); | ||
| var emptyArgs = Utils.EmptyArray<object>(); |
There was a problem hiding this comment.
You call Utils.EmptyArray<object>() multiple times, why not just use s_emptyArgArray?
There was a problem hiding this comment.
I'll remove s_emptyArgArray - it's probably not better to store 2 references to the same array, Utils.EmptyArray already has a static reference.
|
|
||
| $a = [C].Assembly.GetCustomAttributes($false).Where{ | ||
| $_ -is [System.Management.Automation.DynamicClassImplementationAssemblyAttribute]} | ||
| $a.ScriptFile | Should BeExactly (& { $MyInvocation.ScriptName }) |
There was a problem hiding this comment.
Maybe replace (& { $MyInvocation.ScriptName }) with $PSCommandPath?
There was a problem hiding this comment.
Uh, sure. My excuse - I've never used it before and was too lazy to look for it. :)
|
This is great, but the usage seems too complex to remember. Can we use ETS to just add a ScriptFile property making it more discoverable? |
|
Steve Lee (@SteveL-MSFT) - I've only seen 1 request for this ability - from the author of ISESteroids - so ease of use doesn't seem too critical. I suppose we could use a |
|
Jason Shirk (@lzybkr) we can wait for customer request before adding more. Thanks |
Ilya (iSazonov)
left a comment
There was a problem hiding this comment.
Leave a comment
Using the assembly name to hint at the source of the classes was
problematic in multiple ways.
This change stores the actual filename in an attribute on the assembly.
So for a given type, one can get the assembly this way:
[SomeType].Assembly.GetCustomAttributes() |
? { $_ -is [System.Management.Automation.DynamicClassImplementationAssemblyAttribute] } |
% { $_.ScriptFile }
Using the assembly name to hint at the source of the classes was
problematic in multiple ways.
This change stores the actual filename in an attribute on the assembly.
So for a given type, one can get the assembly this way:
[SomeType].Assembly.GetCustomAttributes() |
? { $_ -is [System.Management.Automation.DynamicClassImplementationAssemblyAttribute] } |
% { $_.ScriptFile }