Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

VB -> C#: expression NameOf results in "Cannot convert ClassBlockSyntax" #592

Closed
desbaratizador opened this issue Jul 9, 2020 · 12 comments
Labels
needs-repro Needs either the input that caused the error, or more information to allow reproducing the error VB -> C# Specific to VB -> C# conversion

Comments

@desbaratizador
Copy link

Input code

<ApresentacaoColeccao(New String() {NameOf(Fornecedor.Codigo), NameOf(Fornecedor.Nome)}, 1)>
Public Class Fornecedor
Inherits ClasseDominio

    Public Overrides Function ToString() As String
        Return _nome
    End Function

    Protected _codigo As String
    Public Overridable Property Codigo As String
        Get
            Return _codigo
        End Get
        Set(value As String)
            _codigo = value
        End Set
    End Property

...

Erroneous output

/* Cannot convert ClassBlockSyntax, System.InvalidCastException: Unable to cast object of type 'Microsoft.CodeAnalysis.CSharp.Syntax.EmptyStatementSyntax' to type 'Microsoft.CodeAnalysis.CSharp.Syntax.MemberDeclarationSyntax'.
at ICSharpCode.CodeConverter.CSharp.DeclarationNodeVisitor.d__45.MoveNext()

Input:
    <Global.Logi.Dominio.Atributos.ApresentacaoColeccao(New String() {NameOf(Global.Logi.Dominio.Empresa.Fornecedor.Codigo), NameOf(Global.Logi.Dominio.Empresa.Fornecedor.Nome)}, 1)>
    Public Class Fornecedor
        Inherits Global.Logi.Dominio.ClasseDominio

        Public Overrides Function ToString() As String
            Return Me._nome
        End Function

        Protected _codigo As String
        Public Overridable Property Codigo As String
            Get
                Return Me._codigo
            End Get
            Set(value As String)
                Me._codigo = value
            End Set
        End Property

Expected output

Trying to convert a full project and some classes failing to convert, the patterm is the use of the NameOf operator (when i replace the NameOf with a string constant the conversion goes well), this operator as all know has a direct counterpart in C#.

Details

Using version 8.1.5.0 in Visual Studio.

@desbaratizador desbaratizador added the VB -> C# Specific to VB -> C# conversion label Jul 9, 2020
@GrahamTheCoder
Copy link
Member

Hi, thanks for getting in touch. If I paste the input you gave into the web converter (which uses the same code) it converts fine - so I don't think it's NameOf causing the issue here. It mentions MemberDeclarationSyntax, so it's probably one of the class members that's been cut off when you pasted into Github, could you try again to paste the whole thing so I can investigate, thanks!

@GrahamTheCoder GrahamTheCoder added the needs-repro Needs either the input that caused the error, or more information to allow reproducing the error label Jul 11, 2020
@desbaratizador
Copy link
Author

Hi,

First, thanks.

Tried also the web converter and the conversion was made, apparently exists some divergence between the two tools. Also i remember that when removed the NameOf expressions from the Fornecedor class, achieved a conversion without errors.

See attached the Fornecedor and another VB .NET class and is failed C# conversions, in the new class NameOf seems to be also the problem.

Regards

Desbaratizador
Failed_conversions.zip

@GrahamTheCoder
Copy link
Member

Thanks a lot. I'll check it in visual studio soon.

@GrahamTheCoder GrahamTheCoder removed the needs-repro Needs either the input that caused the error, or more information to allow reproducing the error label Jul 12, 2020
GrahamTheCoder added a commit that referenced this issue Jul 12, 2020
I suspect different visual studio versions are parsing nameof differently. Explicitylt constructing an identifier with the syntax kind may get around this.
@GrahamTheCoder
Copy link
Member

I couldn't reproduce the issue, but looking at the code I can see the current behaviour could differ by Visual Studio version.
I've pushed a change to try to make it more consistent, which will be in the upcoming 8.1.6 release.
It'd be great if you could let me know you visual studio version (from the about box), and try out the new version when it's released.
Thanks!

@desbaratizador
Copy link
Author

Unfortunately the issue remains with version 8.1.6

@GrahamTheCoder
Copy link
Member

Thanks for checking, what visual studio version are you using?

@desbaratizador
Copy link
Author

Microsoft Visual Studio Community 2019 version 16.6.0

@GrahamTheCoder GrahamTheCoder added the needs-repro Needs either the input that caused the error, or more information to allow reproducing the error label Jul 19, 2020
@GrahamTheCoder
Copy link
Member

Could you check if this cut down project works for you?
Issue592And595ReproAttempt.zip

If it works fine, could you try to figure out what needs adding from your full project in order to break it (e.g. project type, target framework)?

@desbaratizador
Copy link
Author

It works fine (for both issues). But the NameOf problem occurs to me again and again, i'am trying to understand what's the difference...

@GrahamTheCoder
Copy link
Member

Thanks!

@GrahamTheCoder
Copy link
Member

GrahamTheCoder commented Jul 22, 2020

The only time I've seen something like this is when the nuget packages weren't restored. I'm going to add something to the converter to warn people to (or automatically) do a full build if references can't be found.
I believe this is also related to dotnet/roslyn#6615

GrahamTheCoder added a commit that referenced this issue Jul 25, 2020
beppe9000 added a commit to beppe9000/CodeConverter that referenced this issue Jul 27, 2020
* Changelog

* Update README.md

* Rough outline of dotnet tool that converts all vb in a solution to c#

TODO:

- MSBuild resolution (probably installing the right packages)
- Args to deal with project files
- Args to deal with overwriting
- Args to deal with different output directory
- Args to force skipping nuget restore
- Args to force continuing with compile errors

Spike of icsharpcode#555

* More args

* Handle msbuild properties and projects

* Rearrange

* SDK version is missing, build against an older one

* Allow use of another directory - fixes icsharpcode#217

* Copying fixups

* Off topic: Use language names

* Use any assemblies hanging around in the app domain too

* Pull out project creation

* Remove cache for comparison

* Improve constructor generation

* Parenthesize cast expressions

* Run in dot net core and characterize reasonable changes

* Use net core types in GlobalImportsStatement

* Add missing using

* Disable check

The versions that come out in .net core are totally different

* Try to preserve conversions to char and array for back-compat

* Block all overwriting unless comitted to git or forced

* Create DirectoryInfoExtensions

* Use MSBuildWorkspaceConverter

* Make static

* Reorder

* Don't dispose, let it be used by other tests since it's immutable

* Attempts useing VS msbuild so system runtime types resolve

* Fix git diff

* Avoid yield because it's a linked file

* Set AnyCPU to give auto-detection a chance

* Try x64

* Run .net core tests

* Spike targeting net48

Need to then figure out if we can use a command line argument to launch the net framework version based on a command line option

If it works in this form, I can pull all the code into a netstandard library which the tests could reference (and even multi-target themselves if we wanted to test with both runtimes)

* Patch in missing behaviour

* Split out tons of parts to get around core vs framework nightmare

* DotNetTool commands must be dot net core
* Dot net core's msbuildlocator uses the net core sdk for workspace creation
  * The net core sdk can't resolve references for framework projects
* Specifically locating msbuild for .net framework throws an error about a missing method - it's not compatible

* Shared project

* Review: Make some things public

I don't really want people from other repos using these since they aren't stable APIs, just helpers. Might need to rethink how to share these

* Revert test framework-related changes

* Call one from the other

* Tell users about this new switch when it inevitably causes pain

Would be better off doing some simple detection on the project files probably.
The framework version of the exe is actually more flexible since it works for framework and core, but it won't work on other platforms. It may make sense to just try to detect when the framework one is runnable.

* Detect environment

* Tell people about the keyboard shortcut for copying

* Convert omitted delegate parameter types using symbol - fixes icsharpcode#560

* Make dot net core version the main named thing and fix up some warnings

* Add a helper script for installing locally

* Restore R# required dependencies

* Revert nullable test changes

* Show errors in tests

* Revert readonly span hacks

The code will only be run against .net core projects which will support them

* Tweak caching to avoid bug in msbuild workspace

* Install nuget.build.tasks to fix workspace level errors

* Recharacterize wrong behaviour (same as start of branch)

Only work in .net core at the moment

* Await file writing task

* Return same extension by default

* Re-set target path after writing

* Update readme

* Wait for the output to be written

* Set working directory in case it helps resolving dependencies

* Use latest version over 16.0 - todo: warn if <16.0 is available

* Load any assembly, and delete source files that aren't in target

* Finalize copy and delegation behaviour

* Avoid passing path to git because it's hard to know what format

* Log loader exceptions

* Give MSBuild locator first go at resolving depenencies + allow others

* Don't copy binaries/caches

* Check happy path

* Tell users when we're ignoring their ancient software

* Only limit git diff lines

* Write exception message last so user sees it in console

* Don't need a stack trace for validation errors that the user can fix

* Avoid huge diffs and issues for non git users

* Bump version to 8.1.0

* Remove appveyor.yml

* Show help on debug install

* Tidy up output when git is missing

* Async test names to avoid warnings

* Fixup most warnings/info items

* Remove async suffix

* Half package size

Turns out they don't need to be content if they're marked as packed. When marked as content it gets included two extra times, and the zip format doesn't seem to compress that well

* Move readme and add design notes

* Add some future notes

* Update README.md

* Remove pretty unused code

* dlls are binaries

* Hack to include dataflow dll for 2017

* Stop binding assembly versions at all, and add version info

* 8.1.1

* Update README.md

* Update bug_report.md

* Deal with array type symbols - fixes icsharpcode#559

* Fix test input compilation

* Parenthesize all binary expression - fixes icsharpcode#565

Simplifier will deal with ones that weren't needed

* Fix NuGet name

* Converting ForEach loop: avoid duplicate variable compilation issue

Fixes icsharpcode#558

* Neaten up case when variable is only used within loop

* Use var except when casting

* Improvements to for loop with missing semantic info - fixes icsharpcode#482

* Loop over enum

* Split byref tests into own file

* Ref improvements

* Lots of WIP for generating a local function

* Rename and add xml doc

* Manually constructed test case

* Widen the hoisting concept

* Create local function (a bit eagerly and with a static name)

* Less eagerly extract local function

* Rename after

* Convert if parts in expected order

* Handle properties and missing semantic info

* Handle other arg types

* Handle type mismatches, constants, instance expressions

* Release notes

* Handle array element references

* Pull out an extra temp for element access expressions

* Add test for object list

* Readability tweaks

* Split out, simplify and neaten the output

* Extract event tests into own class

* Extract property member tests into own class

* Extract operator member tests to own class

* Use stack so nested types get correct initializers

* Tidyups

* Add as constructor initializer

* Deal with initializers not in the part where constructor is created

Fixes icsharpcode#281

* Split arrays with different ranks

Part of icsharpcode#544

* Handle MidAssignmentStatement

Part of icsharpcode#544

* Convert array type when there's an initializer

Part of icsharpcode#544

* Version 8.1.2

* Textual improvements to vsix manifest

* One way of solving test race condition

* Use confirmed thread safe way of creating workspaces

icsharpcode#376 (comment)

* Pass the MSBuild default assemblies

* Convert to async

* Pass through joinable task factory

* Rename -> Async

* Use non-special hex values

* Make switch case labels const where possible

Fixes part of icsharpcode#544

* Change the name to avoid icsharpcode#569

* Extract logic

* Assign names back to their originals - fixes icsharpcode#569

* Remove explicit cast where possible - fixes icsharpcode#388

* If no project file: Hint at installing the VisualBasic nuget package

(for the relevant using statements)

- Relates to icsharpcode#388

* Improve performance of VB->CS single file conversion - fixes icsharpcode#469

Avoid converting My namespace which is thrown away anyway

* Characterize this oddity

* fix typo in readme

* Add AsEnumerable in linq "in" clause - fixes icsharpcode#544

* Tidy up string comparison - fixes icsharpcode#500

* Update changelog

* Update changelog

* Initial copy

* Pull in what's needed to get this building in its current form

* Move in existing mygroup functionality

* Move existing exponentiation logic

* Move existing like operator

* Fix race condition in GetExitCodeAsync().

After `WaitForExitAsync`, the git process has written all its output to the pipes and terminated.
However, it's possible that CodeConverter process has not yet read the data from the pipe, or that `OutputDataReceived` event is still executing after `GetExitCodeAsync()` returns.

This can cause an `ObjectDisposedException` if the caller manages to dispose the `StringWriter` before the data is read from the pipe.

The `OutputDataReceived` event is called once with `e.Data == null` to signify the end-of-output. Waiting for that prevents the race condition.

* Introduce KnownMethod to save inlining 1000s of lines of hardcoding

* Modulo should have conversion

* Subtle behaviour change: Use bool version rather than late bind

* Remove unused using (There's a global import)

* Indexers most commonly have 1 arg

* Use Array.Resize - fixes icsharpcode#501

* Use C# 7.3 compatible null check and simplify unary negation

* Version 8.1.3

* Fix syntax tree issue

* Test tweaks

* Don't qualify non-static generic names

* Don't do any extra qualification for generic names

Doesn't cause any negative effects on tests

* Fix more predefined type name parsing

* Add ref keyword for Array.Resize

* Use correct initializer type

* Don't use private keyword when already declared private

* Nameof expressions contain member access, rather than qualified names

* Update comments

* Update CHANGELOG.md

* Flip condition - a type name should never be a method call

* Fix typo

* Create delegating method for implementations - fixes icsharpcode#443 - fixes icsharpcode#444

* Improve MyClass tests

* Deal with more combinations of explicit / parameters / myclass

* Write to temp path - hopefully fixes icsharpcode#578

* Add version

* Avoid nullref

* Update README.md

* Update CodeConverter.csproj

* Update description -fixes icsharpcode#577

* Update README.md

* Fall back on compilation option

* Dedupe and fix syntaxkind

* Coerce to string in option compare text for char arrays

* Formatting only

* Omit conversion

* Add test with option compare text true

* Cater for nulls too

* Pass through comparison and use invocation style

* Wrap all cases for case insensitive strings

* Deal with case insensitive comparisons

* Try to minimize the number of null coalesces

* Add self verifying tests

* 8.1.4

* Deal with as many combinations of cases as possible

* Changelog

* Use var pattern to reduce scope of variable - fixes icsharpcode#323

* Only cast when switching on enum

* Fix and simplify

* Update CONTRIBUTING.md

* Fix non-string case regression - fixes icsharpcode#585

* Improve numeric casts - fixes icsharpcode#580

* Set refkind for raise event invocations - fixes icsharpcode#584

* Refactor existing renamer

* Rename clashing type members - fixes icsharpcode#420

* Fix string -> enum - fixes icsharpcode#476

* Make comment clearer

* Rename explicit method implementations where needed - fixes icsharpcode#492

* Include type info in conversion of default(someType) - fixes icsharpcode#486

* Do a single field exchange, in case someone else exchanges handlers

* Factor out class to minimize chance of mixing up two handlers

* Report renaming difficulties

* Use workspace options

* Workaround VS caching issue - fixes icsharpcode#586

* Only set if not already set

* Set to the Visual studio JoinableTaskFactory

* If there's no symbol we'd need to write a whole different impl

* Add to changelog

* 8.1.5

* Set file path for text only conversion - fixes icsharpcode#589

* Correct logic for conversion objWithOverloads Is Nothing - fixes icsharpcode#591

* Use object pattern

* Correctly use numeric value - fixes icsharpcode#590

* LangVersion 8

* Increase default language version

* Switch to "is null"

* Correct conversion for equality of overloaded types - fixes icsharpcode#594

* Try to mimimize flakiness due to non-determinism

* Improve heuristic for unknown indexer vs method, fix missing params

Relates to icsharpcode#595

* Attempted fix for icsharpcode#592

I suspect different visual studio versions are parsing nameof differently. Explicitylt constructing an identifier with the syntax kind may get around this.

* 8.1.6

* Revert "Increase default language version"

This reverts commit d66da69.

* Optional parameters in parameterized properties - fixes icsharpcode#597

* Remove unused and fix typo

* Ensure built before conversion

Relates to icsharpcode#592

Co-authored-by: GrahamTheCoder <grahamthecoder@gmail.com>
Co-authored-by: Christoph Wille <christoph.wille@gmail.com>
Co-authored-by: Blubbll <34196146+blubbll@users.noreply.github.com>
Co-authored-by: Daniel Grunwald <daniel@danielgrunwald.de>
@GrahamTheCoder
Copy link
Member

I've forced a build before conversion in the next release, so that might fix this. Reopen if you get a repro for 8.1.7+

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
needs-repro Needs either the input that caused the error, or more information to allow reproducing the error VB -> C# Specific to VB -> C# conversion
Projects
None yet
Development

No branches or pull requests

2 participants