Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Issue with arrays in expression when using DotNetCompilerPlatform #2890

Closed
neoGeneva opened this issue Jul 31, 2015 · 18 comments
Closed

Issue with arrays in expression when using DotNetCompilerPlatform #2890

neoGeneva opened this issue Jul 31, 2015 · 18 comments
Assignees
Milestone

Comments

@neoGeneva
Copy link

Hey, not 100% sure if this an MVC issue or Roslyn issue, but the issue exists when using DotNetCompilerPlatform 1.0.0.0 and MVC 5.2.3.0

The issue is that array variables used in an array get the wrong name (the end up being prefixed with "CS$<>8__locals1.") when used with HtmlHelper expression methods, eg Html.NameFor()

Steps to reproduce:

  1. In VS2015 create a new MVC5 project using the ASP.NET 4.6 template "MVC" (which already has Microsoft.CodeDom.Providers.DotNetCompilerPlatform installed)
  2. Replace the content /Views/Home/Index.cshtml with the razor code below
  3. Start the project and view the result in a browser.
@{
    Layout = null;

    var someArray = new[]
    {
        new
        {
            someProperty = ""
        }
    };
}


<html>
<body>
    @Html.NameFor(_ => someArray[0].someProperty)<br />

    @for (var i = 0; i < someArray.Length; ++i)
    {
        @Html.NameFor(_ => someArray[i].someProperty)<br />
    }
</body>
</html>

The content of the page looks like the following, when the two rows of the output should be identical.

someArray[0].someProperty
CS$<>8__locals1.someArray[0].someProperty
@neoGeneva
Copy link
Author

Note that commenting out the "system.codedom" section in web.config resolves the issue by essentially disabling the DotNetCompilerPlatform package.

@dougbu
Copy link
Member

dougbu commented Jul 31, 2015

MVC 5.2.3 issues are tracked on CodePlex. Have you tried this scenario using MVC 6?

@neoGeneva
Copy link
Author

Yes, I can reproduce the issue in MVC6-beta5, this time using "ASP.NET 5 Preview Templates" -> "Web Application" and change the Home/Index.cshtml view as before.

The result was exactly the same.

@Eilon
Copy link
Member

Eilon commented Jul 31, 2015

@dougbu this is presumably because i is being closed over, which generates a new class, and I'm guessing that's what's showing up here. Does this happen in all closure scenarios when we generate names based on expressions?

@dougbu
Copy link
Member

dougbu commented Jul 31, 2015

Does this happen in all closure scenarios when we generate names based on expressions?

Not all closure scenarios. We have samples and functional tests demonstrating both m => Model[i] and m => m[i] generate the expected names. I believe this happens only when the leftmost expression node is also a local variable. Still important to fix of course.

@dougbu
Copy link
Member

dougbu commented Aug 5, 2015

Problem is not specific to arrays e.g. List<T> instances can hit the same issue. The important things are:

  • Expression's leftmost term is a variable i.e. something other than a property of the view or the lambda parameter.
  • Expression also includes a non-constant indexer term.
  • The two terms change at different times e.g. one is a method variable and the other is a loop variable.

Root cause is our lambda expression-to-name conversion goes too far, treating a field of a compiler-generated class (for the delegate) as just another member expression.

Fix is likely simple: Stop looking for name segments when we reach something that is not a valid C# identifier e.g. contains "<>". But will need to add lots of tests to make sure we aren't missing other related problems.

Code is almost identical in MVC 5 and 6. So the root cause and fix are probably the same, though I haven't tested / debugged MVC 5 as thoroughly.

Details:

The CS$<>8__locals1 portion of the name in the original description appears when the compiler generates more than one class for delegates and they reference each other. E.g.

    private class <>c__DisplayClass28_0
    {
        string[] someArray;
        Asp..._Index_cshtml <>4__this; // reference to containing class
    }

    private class <>c__DisplayClass28_1
    {
        int i;
        <>c__DisplayClass28_0 CS$<>8__locals1;
    }

An instance of <>c__DisplayClass28_1 is passed into the Linq Expression. i is found directly as a field in this class. But someArray is reached via CS$<>8__locals1.

@dougbu dougbu removed the investigate label Aug 5, 2015
@dougbu dougbu removed their assignment Aug 5, 2015
@dougbu
Copy link
Member

dougbu commented Aug 5, 2015

Clearing assignee and Investigate label so Triage can decide on a milestone.

@Eilon Eilon added the bug label Aug 5, 2015
@Eilon Eilon added this to the 6.0.0-beta8 milestone Aug 6, 2015
@Eilon Eilon added the 1 - Ready label Aug 6, 2015
dougbu added a commit that referenced this issue Aug 23, 2015
- #2890
- add lots of `ExpressionHelper` tests using `IdFor()` and `NameFor()` (which are thin veneers)
@dougbu
Copy link
Member

dougbu commented Aug 24, 2015

a045324

@banderberg
Copy link

I am having this same exact issue - it has been driving me nuts. I am using MVC 4.0.20710 with VS 2015. I do not have a "system.codedom" section in my web.config.

To make matters worse I have found that rebooting my system makes the issue go away, but then it mysteriously comes back after an hour or two for no apparent reason.

Is there a work around for this issue?

@dougbu
Copy link
Member

dougbu commented Jan 7, 2016

@banderberg this problem has not been fixed in MVC 5 or earlier, just MVC 6. If you're using MVC 4.x, please report the issue on CodePlex.

FYI the following CodePlex issues are probably related enough that fixing either would fix this problem for MVC 5 as well:

Note to self: Confirm those bugs don't exist in MVC 6.

@mazz0031
Copy link

Is there any fix at all for this in MVC 5.x? Did @dougbu ever verify his 'note to self' to make sure MVC 6 fixes this problem?

I have a project where I am stuck with MVC 5 because MVC 6 is still in release candidate and we're not allowed to upgrade until it is fully released. When .NET 4.6.1 was deployed on our servers we encountered a problem where NHibernate generates an IQueryOver object and started inserting the "CS$<>8__locals1" as a prefix inside the query's objects, which of course fails when it is attempted to be parsed by Linq.

Is there some way to take that IQueryOver object and edit it to remove the CS$<>8__locals1 before passing it into the Linq expression? Is there some way to prevent the CS$<>8__locals1 crap from being generated in the first place?
I tried commenting out the system.codedom section of my config file and that did not fix the problem.

Anything at all that can fix this issue in MVC 5 would be appreciated.

@dougbu
Copy link
Member

dougbu commented Jun 17, 2016

Is there any fix at all for this in MVC 5.x?

MVC 5.2.3 issues are tracked on CodePlex. Did you report the problem there?

In any case I do not believe a newer version of MVC 5.x has been released. No workaround comes to mind.

Did @dougbu ever verify his 'note to self' to make sure MVC 6 fixes this problem?

I may do those checks early in the next milestone. But that's not directly related to your concern. My note to self was about the related MVC 5.x bugs I linked to. The problem described in this bug has been fixed in ASP.NET Core MVC.

we're not allowed to upgrade until it is fully released.

ASP.NET Core MVC RTM is coming soon. See https://github.com/aspnet/Home/wiki/Roadmap

Is there some way to take that IQueryOver object and edit it to remove the CS$<>8__locals1 before passing it into the Linq expression?

The compiler generates these class, property and field names for lambda expressions. Unless NHibernate has approaches or method overloads that avoid lambda expressions, you're probably stuck.

@herrquark
Copy link

I just had to write custom wrapper methods to filter out that kind of stuff. I don't think it will ever be fixed in MVC <= 5

@irega
Copy link

irega commented Jan 20, 2017

Sent a new pull request to fix it on CodePlex:
https://aspnetwebstack.codeplex.com/SourceControl/network/forks/irega/aspnet/contribution/9060

@Eilon
Copy link
Member

Eilon commented Jan 20, 2017

@irega we are no longer tracking PRs to that repo; we are in the process of moving the code to GitHub for a smoother process.

@irega
Copy link

irega commented Jan 21, 2017

Thanks @Eilon, but in Github I only see the .NET Core version. The solution for those who have MVC 5 is to compile their own System.Web.Mvc assembly, dont you?

@Eilon
Copy link
Member

Eilon commented Jan 23, 2017

@irega yup, the GitHub repo is not quite public yet... we just have to dot a few i's and cross a few t's first.

@leolbrj
Copy link

leolbrj commented Feb 5, 2017

@{
    Layout = null;

    var someArray = new[]
    {
        new
        {
            someProperty = ""
        }
    };
}


<html>
<body>
    @Html.NameFor(_ => someArray[0].someProperty)<br />

    @for (var i = 0; i < someArray.Length; ++i)
    {
        @Html.NameFor(_ => someArray[i].someProperty)<br />
    }

    var i = 0;
    @foreach (var item in someArray)
    {
        //ignore item
        @Html.NameFor(_ => someArray[i].someProperty)<br /> //ok
        i++;
    }

</body>
</html>

someArray[0].someProperty
CS$<>8__locals1.someArray[0].someProperty
someArray[0].someProperty //ok

dougbu added a commit to aspnet/AspNetWebStack that referenced this issue Mar 12, 2018
dougbu added a commit that referenced this issue Mar 17, 2018
- tests added (ages ago) in a045324 no longer fail when `__` handling removed
dougbu added a commit that referenced this issue Mar 19, 2018
- tests added (ages ago) in a045324 no longer fail when `__` handling removed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants