modelsbuilder / ModelsBuilder.Original

The Community Models Builder for Umbraco
MIT License
114 stars 49 forks source link

using CSharp 6 features in a partial class results in misleading error on models re-build #151

Closed robertjf closed 6 years ago

robertjf commented 7 years ago

I implemented a property on a partial class taking advantage of CSharp6 expression body property accessors e.g.:

public IQuotientContentModel TargetPage { get => this.UmbracoInternalRedirectId as IQuotientContentModel; }

I have the configuration set up like so:

<add key="Umbraco.ModelsBuilder.LanguageVersion" value="CSharp6" />

and the project is configured to target CSharp 6.2

When I go to build the models, I get the following error:


Failed to build models.
Feature 'expression body property accessor' is not available in C# 6. Please use language version 7 or greater.

at Umbraco.ModelsBuilder.Building.Compiler.ThrowExceptionFromDiagnostic(String path, String code, Diagnostic diagnostic) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\Compiler.cs:line 161
at Umbraco.ModelsBuilder.Building.Compiler.<>c__DisplayClass12_0.<GetCompilation>b__0(KeyValuePair`2 x) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\Compiler.cs:line 50
at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
at Umbraco.ModelsBuilder.Building.Compiler.GetCompilation(String assemblyName, IDictionary`2 files, SyntaxTree[]& trees) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\Compiler.cs:line 46
at Umbraco.ModelsBuilder.Building.CodeParser.Parse(IDictionary`2 files, IEnumerable`1 references) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\CodeParser.cs:line 38
at Umbraco.ModelsBuilder.Umbraco.ModelsBuilderBackOfficeController.GenerateModels(String modelsDirectory, String bin) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Umbraco\ModelsBuilderBackOfficeController.cs:line 108
at Umbraco.ModelsBuilder.Umbraco.ModelsBuilderBackOfficeController.BuildModels() in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Umbraco\ModelsBuilderBackOfficeController.cs:line 35
harvzor commented 7 years ago

I think using an expression body on a get or set is a feature of C#7: https://stackoverflow.com/q/43681882

zpqrtbnk commented 7 years ago

Mmm... so in theory setting the language version to CSharp7 would help... alas the current Roslyn lib we ship does not support it - this will require a new MB release ;(

harvzor commented 7 years ago

@zpqrtbnk I don't understand the complexities of MB, but in my project, I don't even use MB to compile the DLL - I compile it with VS. I just have MB export the generated CS files. Why does it matter what my partial models say then?

I'd much rather catch compilation errors using VS rather than having to look at the Umbraco MB dashboard. Though this isn't a workflow that everyone wants to follow.

Of course, for this issue there's always the option to just not use cutting edge features in our custom partials. 👍

Thanks.

zpqrtbnk commented 7 years ago

MB has various ways to compile models... in PureLive it's using the BuildManager, in Dll mode it's using Roslyn, and of course if you just generate the files then it's whatever you use in VS. For the first two solutions I'm afraid we are currently limited to C# 6.

harvzor commented 7 years ago

@zpqrtbnk I'm using LiveAppData mode on most of my projects. Should this be using Roslyn?

zpqrtbnk commented 7 years ago

LiveAppData means it's just generating the models and you compile it on your own, with VS. So Roslyn is not involved as far as compiling models is involved.

harvzor commented 7 years ago

@zpqrtbnk Could you test that LiveAppData does not involve Roslyn? I'm getting a compilation error on the MB dashboard when I use these config settigns:

<add key="Umbraco.ModelsBuilder.Enable" value="true" />
<add key="Umbraco.ModelsBuilder.ModelsMode" value="LiveAppData" />
<add key="Umbraco.ModelsBuilder.ModelsDirectory" value="~/../Umbraco.Web.PublishedContentModels" />
<add key="Umbraco.ModelsBuilder.AcceptUnsafeModelsDirectory" value="true" />

What I have found is that MB deletes any generated files, then errors due to my custom models requiring C#7. It then fails to create new models so I can't compile with VS.

Thanks.

harvzor commented 7 years ago

I also get an error with the above settings if I use an expression body property such as:

public string Foo => "bar";

The error is:

Failed to build models.

Feature 'expression-bodied property' is not available in C# 5.  Please use language version 6 or greater.

   at Umbraco.ModelsBuilder.Building.Compiler.ThrowExceptionFromDiagnostic(String path, String code, Diagnostic diagnostic) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\Compiler.cs:line 161

   at Umbraco.ModelsBuilder.Building.Compiler.<>c__DisplayClass12_0.<GetCompilation>b__0(KeyValuePair`2 x) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\Compiler.cs:line 48

   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()

   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)

   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)

   at Umbraco.ModelsBuilder.Building.Compiler.GetCompilation(String assemblyName, IDictionary`2 files, SyntaxTree[]& trees) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\Compiler.cs:line 46

   at Umbraco.ModelsBuilder.Building.CodeParser.Parse(IDictionary`2 files, IEnumerable`1 references) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Building\CodeParser.cs:line 38

   at Umbraco.ModelsBuilder.Umbraco.ModelsBuilderBackOfficeController.GenerateModels(String modelsDirectory, String bin) in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Umbraco\ModelsBuilderBackOfficeController.cs:line 108

   at Umbraco.ModelsBuilder.Umbraco.ModelsBuilderBackOfficeController.BuildModels() in X:\Projects\Umbraco\ModelsBuilder\Zbu.ModelsBuilder\Umbraco.ModelsBuilder\Umbraco\ModelsBuilderBackOfficeController.cs:line 50
arknu commented 6 years ago

Just hit this as well, running Umbraco 7.7.9 with this config:

<add key="Umbraco.ModelsBuilder.Enable" value="true" />
<add key="Umbraco.ModelsBuilder.ModelsMode" value="AppData" />
FransdeJong commented 6 years ago

It looks like updating the nuget package Microsoft.CodeAnalysis.CSharp to the latest version solves the problem. Please use at your own risk. I only checked this with Modelsbuilder Api.

zpqrtbnk commented 6 years ago

Right, what happens is that MB not only generates code, it also parses code - reading your partials, which may contain attributes that could control generation. For instance, if you provide an implementation of a property in a partial, we have to detect it in order not to generate the property.

In AppData mode, we don't compile anything but we still parse your partials, and I believe this is where the error occurs. So you cannot use C#7 there either.

Solution would be to upgrade Roslyn (see #106) but that comes with its own set of problems. Not planned in a very near future.