Done.

#Closed

It allows compile a source code on PowerShell Core and Windows PowerShell with different set of usings. Sometimes .Net Core and .Net Framework is different in that.

But the Add-Type in Windows PowerShell doesn't support this parameter in FromSource set, and thus it won't bring us this benefit. I would rather have UsingNamespace only belong to FromMember set, just like today.

I believe the scenario can be important if someone has a portable code template and have to compile in Windows PowerShell and PowerShell Core with different usings. If the scenario is important we should enhance Windows PowerShell too. If no I'll remove this.

Fixed.

Yes, will do. (I wanted reduce size of the PR)

Done. #Closed

Is -PassThru available in all parameters set? If so, then the original [Parameter()] is more preferred as it's easy to tell that a parameter is in all parameter sets.

The membership is under question. I want to get feedback. If no one objects to this '-PassThru available in all parameters set ' then I will use [Parameter()].

Currently it is not in "FromAssemblyName".
I think it makes sense to add?

Add-Type -AssemblyName F:\pscore\Microsoft.PowerShell.Commands.Utility.dll -PassThru returns all available types from the DLL, so I think we should keep the same.

This works.

I think currently Add-Type -TypeDefinition $code -OutputAssembly f:\test.dll only produces the assembly without loading it. So is this new parameter really needed?

We should always load assembly by default otherwise it is a breaking change.

Add-Type -TypeDefinition $code -OutputAssembly f:\test.dll doesn't load the assembly today, so why would it be a breaking change?

I tried the following in PSCore:

[System.AppDomain]::CurrentDomain.GetAssemblies() | Out-File F:\old.txt
Add-Type -TypeDefinition $code -OutputAssembly F:\test.dll
[System.AppDomain]::CurrentDomain.GetAssemblies() | Out-File F:\new.txt

One assembly is loaded when calling Add-Type, and it's System.ValueTuple.dll. I don't see the test.dll being loaded.

Fixed

Will this handle the case that the code in those files references to symbols defined in other files?

My understanding is yes - it is the same parser/compiler session. But I don't test this.

Can you please verify it? ParseText is a static method, so it's hard to say for sure that one signle parser session is used for all those files.

Done. Works well.

Add-Type -Path C:\tmp\1.cs,c:\tmp\2.cs
[Test.AddType.CSharpTest2]::Add2(1,2)
3

1.cs

namespace Test.AddType
        {
            public class CSharpTest1
            {
                public static int Add1(int a, int b)
                {
                    return (a + b);
                }
            }
        }

2.cs

namespace Test.AddType
        {
            public class CSharpTest2
            {
                public static int Add2(int a, int b)
                {
                    return CSharpTest1.Add1(a, b);
                }
            }
        }

nit: By looking at this assertion here, I feel maybe arrange this if {} else if {} else if {} else {} in a switch of parameter name would be cleaner.

Done. #Closed

I think it would be better if you make _syntaxTree also a local variable. It can be passed to IsSourceCodeUpdated and CSharpCompileToAssembly.
I think it's better to make it clear what a method consumes and produces in its signature, and having the arguments like compilationOptions, emitOptions, syntaxTree passed into methods helps it.

I am waiting #6141 (comment)

Fixed.

Similar comments as in CSharpSourceCodeProcessing.

Fixed.

Do we honor -PassThru in this case?

Yes, see below in DoEmitAndLoadAssemply

Done.

It allows compile a source code on PowerShell Core and Windows PowerShell with different set of usings. Sometimes .Net Core and .Net Framework is different in that.

Yes, will do. (I wanted reduce size of the PR)

errorPosition is not used anywhere. Do you intend to use it to replace lineSpan.StartLinePosition.Character at line 1397?

Yes, thanks - it's more readable.
Done.

I don't see the change that addresses this comment in your new commit.

Sorry, lost commit.
Fixed.

Maybe change to if (!char.IsWhiteSpace(errorLineString[i]))? It's more readable.

I thought about it but there's a lot of other characters
https://msdn.microsoft.com/en-us/library/t809ektx%28v=vs.110%29.aspx
Is it ok?

As long as they are whitespace characters, I don't see a difference. Do you?

I fixed this but ...
This come from original code and I'm afraid the code isn't exactly correct because we're replacing one tab with a single space. Perhaps it was necessary for CodeDom. I hope that Roslyn don't use tabs in error messages. Although these whitespaces can come from the code that it compiles. Therefore we have to clean the error message too or remove this cleanup at all. Thoughts?

Fixed.

Math.Max(0, lineSpan.StartLinePosition.Character - i)

Is the Math.Max call really needed? It seems to me that errorPosition would be -ge i.

When I researched how to make the error report I was looking for code examples. Now I do not remember where I saw this pattern.
I think this is because the lineSpan and the error position are completely different Roslyn code paths - so we should protect.

For CSharp or VB?

VisualBasic
Fixed. #Closed.

I guess this applies to both csharp and vb. If that's the case, for CSharp should be removed.

I don't found the option for VB https://github.com/dotnet/roslyn/blob/master/docs/compilers/Visual%20Basic/CommandLine.md

OK, then never mind. #Closed.

But the Add-Type in Windows PowerShell doesn't support this parameter in FromSource set, and thus it won't bring us this benefit. I would rather have UsingNamespace only belong to FromMember set, just like today.

https://github.com/PowerShell/PowerShell/pull/6141/files#r171029964
I want to point it out again that this is a breaking change in that System and System.Runtime.InteropServices are not included when -UsingNamespace is not specified after this change.

If we add this we get errors:

>  $guid = [Guid]::NewGuid().ToString().Replace("-","")
>         $CSharpCode1 = @"
>> using System;
>> using System.Runtime.InteropServices;
>>         namespace Test.AddType
>>         {
>>             public class CSharpTest1$guid
>>             {
>>                 public static int Add1(int a, int b)
>>                 {
>>                     return (a + b);
>>                 }
>>             }
>>         }
>> "@
PS C:\Users\sie\Documents\GitHub\iSazonov\PowerShell> Add-type -TypeDefinition $CSharpCode1
Add-type : (2,1): hidden CS8019: Unnecessary using directive.
using System.Runtime.InteropServices;
^
At line:1 char:1
+ Add-type -TypeDefinition $CSharpCode1
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : InvalidData: ((2,1): hidden C...sing directive.:CSDiagnostic) [Add-Type], Exception
+ FullyQualifiedErrorId : SOURCE_CODE_ERROR,Microsoft.PowerShell.Commands.AddTypeCommand

Add-type : (1,1): hidden CS8019: Unnecessary using directive.
using System;
^
At line:1 char:1
+ Add-type -TypeDefinition $CSharpCode1
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : InvalidData: ((1,1): hidden C...sing directive.:CSDiagnostic) [Add-Type], Exception
+ FullyQualifiedErrorId : SOURCE_CODE_ERROR,Microsoft.PowerShell.Commands.AddTypeCommand

Currently Appveyor CI failed with the same error in log line 3827. 😕

Can I rebase to fix an error with File.wxs?

The error seems caused by a warning that more than one using system; is specified. I'm sure there is a way to deal with it.

Feel free to rebase.

There are two problems here:

1. The assert at this location doesn't do anything useful because when the list is initially created, its count will be 0.
2. I don't know how many people uses debug mode binaries. It could happen that this assert fails without us knowing. This happened before to an assert in NativeComamndParameterBinder.

What if -CompileOnly and -PassThru are both specified?
As mentioned in some other places, I don't think we need this parameter.

Currently -PassThru will be ignored.
The switch came from #5725 (comment)

As I stated in comments above #6141 (comment), when using -OutputAssembly, Add-Type only compiles the assembly without loading it. So I don't think -CompileOnly is really needed.

Currently we always load generated assembly. So with -CompileOnly we address new scenario.

Fixed.

Based on the comments in SyntaxTree, you can directly call ToString() without having to call GetText() first.

Fixed. #Closed

IAssemblySymbol has a property TypeNames, of the type ICollection<string>. Maybe we can directly use that property to avoid a visitor (simplify the code)?

Seems we have to use the recursion dotnet/roslyn#6138

The linked issue is quite old (2015), and it doesn't mention the IAssemblySymbol.TypeNames property. If it were me, I would try this property to see if it works.

Tested - IAssemblySymbol.TypeNames doesn't return full type names.

AsParallel() is likely to be less performant when it comes to a small collection. In the case of Add-Type, most usages involve only a few types being compiled, so AsParallel might not serve our interest here.

If we use the property IAssemblySymbol.TypeNames, then we can decide whether to use AsParallel based on the Count (the property returns an ICollection of string).

I'd remove the AsParallel at all but but don't found how.

If IAssemblySymbol.TypeNames works, we won't need the visitor code :)

AsParallel() was replaced with foreach.
Fixed.