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

Review IsTopLevelStatementsEntryPointMethod implementation #4253

Closed
1 of 2 tasks
Youssef1313 opened this issue Sep 30, 2020 · 2 comments
Closed
1 of 2 tasks

Review IsTopLevelStatementsEntryPointMethod implementation #4253

Youssef1313 opened this issue Sep 30, 2020 · 2 comments
Labels
help wanted The issue is up-for-grabs, and can be claimed by commenting

Comments

@Youssef1313
Copy link
Member

Youssef1313 commented Sep 30, 2020

This was implemented in https://github.com/dotnet/roslyn-analyzers/pull/4066/files#diff-b16e801c575072ba829f30b96c9d2362R692-R694

The current implementation:

        public static bool IsTopLevelStatementsEntryPointMethod([NotNullWhen(true)] this IMethodSymbol? methodSymbol)
            => methodSymbol?.ContainingType.IsTopLevelStatementsEntryPointType() == true &&
               methodSymbol.IsStatic &&
               methodSymbol.Name switch
               {
                   "$Main" => true,
                   "<$Main>$" => true,
                   _ => false
               };

The generated entry point for top-level is "<Main>$" which isn't covered by the method.

Actions needed

@Youssef1313
Copy link
Member Author

Youssef1313 commented Sep 30, 2020

@mavasani I debugged this. It seems like the name we get is $Main, without any <>.
Any idea where the angle brackets have gone? 😄
Edit: the compiler had the name as $Main, but that was changed in dotnet/roslyn#45930 to <Main>$.

@Evangelink Evangelink added the help wanted The issue is up-for-grabs, and can be claimed by commenting label Dec 21, 2020
@Youssef1313
Copy link
Member Author

I'm going to close this as the implementation was corrected. Using Roslyn API isn't possible because we are not using a recent enough version of Roslyn, and I think adding something to Lightup for this case doesn't worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

No branches or pull requests

3 participants