Skip to content

Fix initialization of FakeProperty with setter #2792

Merged
siegfriedpammer merged 3 commits intoicsharpcode:masterfrom
exyi:fix-FakeProperty-setter
Nov 3, 2022
Merged

Fix initialization of FakeProperty with setter #2792
siegfriedpammer merged 3 commits intoicsharpcode:masterfrom
exyi:fix-FakeProperty-setter

Conversation

@exyi
Copy link
Copy Markdown
Contributor

@exyi exyi commented Sep 29, 2022

ILSpy crashed with NullReferenceException when it couldn't resolve some dependency (Newtonsoft.Json in my case, the problem was around JObject.Item[string] indexer)

Seems like a typo when FakeProperty is initialized - the setter method was assigned to the Getter property

Solution

First commit contains the actual fix.

In the second commit I included some asserts and a ToString implementation I used to debug the problem. I'm guessing that more asserts would be welcome, but if you don't like it, just merge only the first commit.

  • At least one test covering the code changed
    • Sorry, but I couldn't make the tests run on Linux, is there any way?
    • I also don't know how to replicate this problem in unit tests - I'd have to compile a dll against some library which the decompiler won't see at runtime.

@siegfriedpammer
Copy link
Copy Markdown
Member

  • I also don't know how to replicate this problem in unit tests - I'd have to compile a dll against some library which the decompiler won't see at runtime.

You could create an IL-Pretty-Test like this one: https://github.com/icsharpcode/ILSpy/blob/master/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/UnknownTypes.il

ILAsm will happily assemble a file that contains references to unknown types/members.

Seems like a typo - the setter method was assigned to the Getter property

The test GuessAccessors needed adjustments in the generated code.
ILSpy is more eager to merge property assignments
@exyi exyi force-pushed the fix-FakeProperty-setter branch from cdb428f to 44a6aa7 Compare October 5, 2022 14:22
@exyi exyi force-pushed the fix-FakeProperty-setter branch from 44a6aa7 to 93eecf9 Compare October 9, 2022 09:29
@exyi
Copy link
Copy Markdown
Contributor Author

exyi commented Oct 9, 2022

I added a test case which I believe covers the changed code.

I also needed to slightly change output of one the existing test. It does not change the meaning of the code, but it's now eager to merge assignments into var v = (x.Prop=c.Prop) expression and then it does not know how to name the variable v.

@siegfriedpammer siegfriedpammer merged commit 6dc55eb into icsharpcode:master Nov 3, 2022
@siegfriedpammer
Copy link
Copy Markdown
Member

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants