GeezerVison

TBH, I would have liked this PR to be part of the 0.10 release as IMO the mono engine is still restricted without it. However, this is a big change so it’s also a bit risky. That’s why I haven’t been pushing for it. Unfortunately I created it after the code freeze.

The current engine isn’t really user friendly as NRefactory can screw up the parsing if code contains blocks and give unexpected messages. Also often the line error messages are incorrect and exceptions not passed to be handled by ScriptCs.Core. This will be annoying for users scripting on the engine.

As code speaks loader then words here are some code snippets.

Block parsing failure
This will error in current engine put evaluate correctly with this PR

class A { public string X { get { return "Class A"; } } }
if(true) { var x = 10; } 
class B { public string X { get { return "Class B"; } } }
Console.WriteLine(new A().X);
Console.WriteLine(new B().X);

Issue #713
This will error in current engine put evaluate correctly with this PR

var x = 123;
Action a = () => x++;
Console.WriteLine(x);

REPL multiline
This will not work in the current engine but with this PR
> Console.
* Writeline(“Hi”);

Errors

var a = 1;
var b = 2;
Foo(a,b);
public void Foo(int a, int b) { return a + b; }

Current engine will give error message at line 2,2
(2,2): error CS0127: `System.Action<int,int>': A return keyword must not …

While the new engine will return more correctly
ERROR: (4,34) CS0127 `System.Action<int,int>': A return keyword must not …

Also this PR will build bau scripts … checked both bau and bau-msbuild
https://github.com/bau-build/bau/blob/dev/baufile.csx
https://github.com/bau-build/bau-msbuild/blob/master/baufile.csx

Just my 5 cent for now!
Cheers

this is awesome.

I would suggest that MonoReportPrinter is submitted as a separate PR - we try to keep PRs as lean as focused as possible, and it would make sense to isolate that one. I was actually working on the printer today too but it's even better that you got there first :)

I would suggest the same for REPL multiline, but this looks like it's too coupled to the new code.

Makes sense to you?

@filipw ... yes I know it's a big block! There are many issues interconnected here but I tried to keep he commits self contained. I can't see why I couldn't extract both MonoPrinter and MuliLine REPL into a separate PR . But now I'm off to watch the game :)

@ztone hey man, do you mind picking this up like we discussed? It would be good to have 3 separate PRs here:

• printer
• multiline
• lexer

This way it would be easier to digest and error-proof. Thanks again, since this work is extremely valuable.

If you need any help let me know.

Hi @filipw

I've changed the PR to just include the lexer, which is the bulk of my former PR anyway.
The goal with this lexer is to get up to par with Rosyln. Error reporting will still be off as I've removed the MonoReporter.

Cheers,
Stone

@filipw are you looking into this?

@filipw @adamralph I rebased this PR.

Once I took a closer look I remembered that this PR has some needed features like keeping track of line numbers (as scripts are being re-ordered before evaluation) and maintains code format as classes and methods are re-formatted by NRefactory in our current solution. This is needed when we want to add our custom mono reporting class. Also because of this reformatting, implementing multiline support for classes is going to be awkward in our current solution. For example, NRefactory will add a missing curly bracket for classes which will mean that the Mono.CSharp will not think it’s an incomplete submission.

Pros:

• Fixes block parsing errors
• Fixes Expressions capturing local vars do not compile in Mono #713 as the newest Mono.CSharp does not have that fix yet.
• Keeps track of line-number (to be used with the mono reporter)
• Keep code format intact so it will be easier to add multi-line support for classes.

Cons:

• This is a large chuck of code that might contain unforeseen parsing bugs, but is has a great deal of unit-tests.
• A small performance hit as script needs to be pre-parsed.

great. IMHO this is good to merge. I know this is a massive changeset, but it was expected to be, since it introduces a lexer. I wouldn't worry about introducing bugs that much. Firstly, it has a bunch of tests like you mentioned. Secondly, I tested it out when it was brought forward originally and it worked like a charm.

I say this should go in immediately, as it's definitely an improvement in our Mono support. unless someone wants to chime in? @scriptcs/core

Thanks @ztone! This will go out in 0.13.