Skip to content
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

Fix Various PowerShell Class Issues #6652

Open
8 of 23 tasks
rjmholt opened this issue Apr 14, 2018 · 28 comments
Open
8 of 23 tasks

Fix Various PowerShell Class Issues #6652

rjmholt opened this issue Apr 14, 2018 · 28 comments
Labels
Issue-Meta an issue used to track multiple issues KeepOpen The bot will ignore these and not auto-close WG-Engine core PowerShell engine, interpreter, and runtime WG-NeedsReview Needs a review by the labeled Working Group
Milestone

Comments

@rjmholt
Copy link
Collaborator

rjmholt commented Apr 14, 2018

Meta-issue to track work on improving the experience of PowerShell class usage as much as possible.

Note on By-Design Behaviours

PowerShell classes currently are a bit finnicky. Some of this is a necessary pain because of the design constraints behind classes.

Classes are currently implemented as compiling to .NET IL so that we can take advantage of the .NET object model. Not doing this would mean us reinventing (and having to maintain) the wheel on essentially all object-oriented features available in .NET (or anywhere else) and paying a high runtime overhead on shimming compatibility with the .NET object model (e.g. faking inheritance from .NET classes).

Instead, we compile PowerShell classes to dynamic assemblies and bake in calls to PowerShell in the generated IL. To do this at runtime would mean either breaking dynamic- and module-scoped behaviours in classes, or emitting a new dynamic assembly every time we hit a class definition (and since PowerShell's highest compilation unit is the scriptblock, and scriptblocks are permitted in for-loops, the performance penalty here could be extreme). (I'm still a bit hazy on the details about the mechanisms here, especially in terms of why caching is made impossible by module scope or in comparison to Add-Type, but @daxian-dbw or @lzybkr will be able to add to/correct me).

The need to compile classes at parse-time means the types required to define a class (in IL) must also be known at parse-time. The module-scoping issue means that a using module statement is required to import classes from modules (@daxian-dbw might like to add information here about the specifics governing this need), or by exporting class usage in PowerShell functions (classes having a sort of module-private behaviour).

Issues to Improve

There are, however, a few improvements possible to PowerShell classes that might not run up against the design constraints given above. Below is a list of open issues for classes in PowerShell.

Classes in modules

Type errors

Other bugs

New feature requests

A couple of other resources on these issues:

@sancarn
Copy link

sancarn commented May 5, 2018

Have override methods been suggested?

For example I have had to implement GUIs while overriding the WndProc method before. So a way to override methods would be a great addition to Powershell classes.

Edit:

I noticed #6418 suggested overriding property setters and getters. I suppose that is similar to this.

@christru
Copy link

Is there any community projects that work around these issues for example? I cannot for the life of me get a decent project implemented in PS w/ classes broken out into a sensible manner without running into one of the above issues..

@devblackops
Copy link
Contributor

@christru You can take a look at my PoshBot module here: https://github.com/poshbotio/PoshBot

That module is almost entirely PowerShell class based.

@christru
Copy link

@devblackops as I’m litterally watching your YouTube video talk on classes. Thanks buddy!

@devblackops
Copy link
Contributor

@christru Cool. I did the same talk at the PowerShell Summit. It is a more polished version. https://www.youtube.com/watch?v=i1DpPU_xxBc&list=PLfeA8kIs7CocGXuezOoYtLRdnK9S_Mq3e

@IISResetMe
Copy link
Collaborator

@rjmholt I believe #8302 (classes don't produce valid interface property methods) should be filed under "Other bugs"

@rjmholt
Copy link
Collaborator Author

rjmholt commented Nov 22, 2018

@IISResetMe currently classes don't allow overrides. Wouldn't the virtual addition be part of the interface inheritance?

@IISResetMe
Copy link
Collaborator

@rjmholt well, we already mark all generated methods virtual, allowing class definitions to inherit existing interfaces:

class PleaseDisposeOfMe : IDisposable
{
  [void]Dispose(){ <# free unmanaged resources #> }
}

but since we never mark the underlying get/set methods of properties virtual (looks like an oversight), you can't actually implement an interface with a property:

class MyPrincipal : System.Security.Principal.IPrincipal
{
  [System.Security.Principal.IPrincipal]$Identity
  [bool]IsInRole()
}

At line:2 char:1
+ class MyPrincipal : System.Security.Principal.IPrincipal
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Error during creation of type "MyPrincipal". Error message:
Method 'get_Identity' in type '<404e3736>.MyPrincipal' from assembly '⧹powershell, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' does not have an implementation.
    + CategoryInfo          : ParserError: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : TypeCreationError

@KirkMunro
Copy link
Contributor

KirkMunro commented Dec 5, 2018

Forgive some ignorance on my part, but looking at the number of issues and the complications that exist in classes in PowerShell today, I can't help but feel this is a design problem rather than a collection of issues/bugs to be addressed.

Wouldn't it make more sense and be much easier if:

  • PowerShell had a new module type specifically for class definitions, called something like class module that could only include class definitions, with a psx1 file extension; nothing else would be allowed in these files (no functions, variables, command invocations, etc.)
  • Import-Module automatically recognized psx1 files defined in RootModule, or class modules included in NestedModules, or RequiredModules fields in a module manifest, as well as psx1 modules that don't have a manifest at all, and loaded the types defined in those psx1 files accordingly (RequiredModules first, then NestedModules, then RootModule, so that you can derive from other types in other modules)
  • PowerShell automatically identified classes derived from PSCmdlet and added them to the current session without the extra work that is being done here.
  • PowerShell automatically exported classes that are associated with cmdlets that are exported from a module.
  • PowerShell supported ClassesToExport in module manifests to identify types in modules so that implicit loading could still work, even with classes.
  • Export-ModuleMember had a -Class parameter to explicitly identify which classes you want to export

One of the drivers behind this approach is that it would do away with the need for using for modules containing classes, and supports #requires, NestedModules, RequiredModules, which makes installing modules from the PowerShell Gallery work as expected, and everything else that people already do with Import-Module today. Having both using and Import-Module is confusing and already requires a lot of re-thinking when it comes to how you set up modules to properly load classes. It also facilitates defining an equivalent of binary modules from within PowerShell itself.

There are more thoughts behind this, but I've shared enough to see what others think for now.

@KirkMunro
Copy link
Contributor

KirkMunro commented Dec 5, 2018

On second thought, I'd prefer not having to deal with Export-ModuleMember or ClassesToExport as suggested in those last two bullets at all, having psx1 files be a PowerShell-ish equivalent of a type library, with types automatically exported just as they are with dlls (ideally with something resembling public/private to control visibility). With that approach, the last three bullets wouldn't be necessary -- classes would just be loaded and available once a module was imported.

@lzybkr
Copy link
Member

lzybkr commented Dec 5, 2018

My random thoughts:

  • Not adding ClassesToExport was intentional, and type are public to help make the interactive experience good.
  • I'm skeptical that auto-loading types is a good idea, at least as a default.
  • Defining cmdlets via classes is useful and doesn't require a new module type.
  • Not being able to define functions seems a bit harsh, but I like the intention. In a new module type we could have strict functions, and these functions could even be compiled like cmdlets, convert parameters to properties, etc. This isn't exactly easy, but I think it's fairly mechanical.

@Jaykul
Copy link
Contributor

Jaykul commented Dec 5, 2018

Yeah, clearly it would be a great thing if Importing a module automatically included Cmdlet-derived classes as commands.

But that circles around to Import-Module and also using module -- needing to explicitly using a module, even though I've already explicitly imported (or Nested or Required) it is one of the biggest pain point with classes for users.

@lzybkr I don't quite understand why you think leaving off the ClassesToExport feature and requiring me to export them by outputting them (or the user to import them with a second line of code) is a good thing: binary modules do this as a matter of course, I mean, I don't need that to result in auto-loading, but I want Import-Module to be enough so that every class I use as a parameter type (or a property of a parameter type) is available to the user.

Of course, I also think we should actually namespace the classes by module (i.e. [ModuleName.ClassName]) -- that would certainly become critical if you started thinking about auto-loading / implicit loading.

@lzybkr
Copy link
Member

lzybkr commented Dec 6, 2018

Uh, I wasn't suggesting "outputting them" either - you define a class in a psm1, it's exported. All classes are exported, simple as that.

I was never a fan of the *ToExport properties in the manifest, it feels like it violates the DRY principle. As an optimization, it seems fine, but I'd rather it be a build artifact.

And classes are modeled more like an assembly - public types don't require any entries in the module manifest.

The need for Import-Module and using module isn't really by design, it's more like we didn't have enough time to work out all the issues and people found workarounds that we live with because it seems to work. Obviously the intention was to have a more static world that you could reason about.

@vexx32
Copy link
Collaborator

vexx32 commented Dec 16, 2018

Any chances we could get constructor chaining on the radar at some point, too? 😄

@fMichaleczek
Copy link

fMichaleczek commented Apr 23, 2019

@rjmholt rjmholt One of my old issues about classes (from MS Connect) was not on your, so I search all issues open (missed + new) and compare with your list :

#1760 Inheritance from class with abstract property is inconsistent
#1751 format-* cmdlets cannot display hidden class members
#2217 Allow (some) method names that happen to be keywords
#2219 Properties with accessor and mutator methods
#2225 Comment-based Help for classes
#2841 PowerShell class defined in 'New-Module -ScriptBlock' doesn't work as expected
#2876 Enable native interop with static extern class methods
#4113 Running using module f:\tmp\test in global scope doesn't load the powershell class defined in the module to the global scope, while using module f:\tmp\test\test.psm1 does
#4713 PowerShell class methods cannot invoke non-exported functions.
#5332 Inheritance from interface and class are inconsistent
#5392 Suggestion: Implement Yield Return for Class Methods
#5796 Method parameter attributes are ignored
#6722 Type checking in PowerShell class method bodies is not needed
#7287 Custom classes and enums are not recognized by tab completion
#7294 Classes: an uninitialized [string] property defaults to $null rather than the empty string #7294
#7506 Permit specifying parameter names for constructors / .NET methods
#7654 "Using module" statement does not reload module after changes are made _(Note : We are forced to used the keyword using to import properly another module)
#7736 When powershell class method has same name as property, the property disapear and is not accessible from any instance
#8235 Class : Methods and properties can't have the same name
#8475 using module fails to check already-loaded modules for available custom types #8475
#8767 [BUG] Custom classes can be redefined in the same scope, via dot-sourcing, take effect in delayed fashion
#8828 class based DscResource requires inherited class to be in same file
#9106 keywords in class method names meaningless error message
#9174 Custom class methods do not complain about an unassigned $args variable
#9313 PowerShell classes leak to other runspaces on macOS #9313
#9445PowerShell class syntax doesn't support multiple conversion operators with the same input type

@fMichaleczek
Copy link

fMichaleczek commented Apr 23, 2019

I began to report classes issues with PS 5.0 Preview April and nothing has changed in 4 years. (no interface, forbidden methods names not consistant, override isn't implemented , bugged using...)

From my point of view, there is only one solution to solve this big issue :

  • Classes with function support with a limited scenario (Class parser to IL)
  • SuperClasses without function support but able to manage a runspace/runspace pool (Class parser to assembly)

This SuperClass need a very big improvement in the parser because we don't want another limited classes. To be clear, I want a class parser like PSLambda for Roslyn.

I don't know if, writing a new extended parser and asking Roslyn to do the job, is more difficult than resolved all these issues. But we are all waiting for a real statement from PowerShell comitee around classes.

@SteveL-MSFT To be honested, the aim behind classes are not to write 2 or 3 + 10 methods ... functions already do that ! Can we take a full scenario to determine priority ? I've got one if you want : AspNet Core on PowerShell. (even IronPython had this scenario in net4)

I want to resolve this issue in priority because it's very very dangerous : #5332

@iSazonov
Copy link
Collaborator

For reference #9382. Maybe we could make friends with these areas.

@msftrncs
Copy link
Contributor

@rjmholt, I think #8028 has been completed and closed now as of PS Core 6.2.

@ili101
Copy link

ili101 commented Aug 26, 2020

Hi. Can you please add Named and Optional Arguments (C# Programming Guide) to the list?
Optional: #9701 #7534
Named: #13520 #13307

@JustinGrote
Copy link
Contributor

JustinGrote commented Jan 25, 2022

Didn't see this in the issue list, I can create a separate issue:

The array representation of a class (e.g. Mything[]) can't be referenced if the class definition is in another file.

#Class.ps1
class Test {[String]$Name}
#Usage.ps1
. $PSScriptRoot/Class.ps1
$myTest = [Test]@{Name='Yay'}
[Test[]]$myTest

Will error with ArgumentException: Could not find type [].. Put them in the same file and it works fine however.

EDIT: This was actually more of a Pester thing, -BeOfType doesn't work with Powershell Classes. Workaround: Use $item.GetType().Name | Should -Be 'ClassName'. This should still work natively ideally so leaving it here.

@iSazonov
Copy link
Collaborator

@JustinGrote Please open new issue to discuss your scenario. Then I could add the issue to the tracking list.

@vexx32 vexx32 added the WG-Engine core PowerShell engine, interpreter, and runtime label Aug 8, 2022
@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-No Activity Issue has had no activity for 6 months or more labels Nov 16, 2023
@ForNeVeR
Copy link
Contributor

But this is a meta issue. What activity is expected?

@JustinGrote
Copy link
Contributor

@ForNeVeR @SteveL-MSFT
Just a casualty of an automated Process. Steve can you revert this?

@mklement0
Copy link
Contributor

@StevenBucher98 StevenBucher98 removed the Resolution-No Activity Issue has had no activity for 6 months or more label Nov 28, 2023
@JustinGrote
Copy link
Contributor

@StevenBucher98 A lot of the issues on this rollup are now marked as closed. Another casualty of this "close everything" policy. At the very least, could the close bot close them as "not planned" rather than "completed?"

@mklement0
Copy link
Contributor

Another one to add to the list (a using module problem with referencing custom class definitions in the the param(...) block, specifically).

@JustinGrote
Copy link
Contributor

A note to this megathread that most of these issues are not actually fixed, they were just closed by the autobot.

@SteveL-MSFT SteveL-MSFT added KeepOpen The bot will ignore these and not auto-close WG-NeedsReview Needs a review by the labeled Working Group labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Meta an issue used to track multiple issues KeepOpen The bot will ignore these and not auto-close WG-Engine core PowerShell engine, interpreter, and runtime WG-NeedsReview Needs a review by the labeled Working Group
Projects
Developer Experience/SDK
  
Awaiting triage
Development

No branches or pull requests