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

VB -> C#: Missing event wiring for overridden members #774

Closed
plalonde4321 opened this issue Oct 6, 2021 · 10 comments · Fixed by #802
Closed

VB -> C#: Missing event wiring for overridden members #774

plalonde4321 opened this issue Oct 6, 2021 · 10 comments · Fixed by #802
Labels
VB -> C# Specific to VB -> C# conversion

Comments

@plalonde4321
Copy link

plalonde4321 commented Oct 6, 2021

VB.Net input code

Private Sub btn2_Click(sender As Object, e As EventArgs) Handles btn2.Click
        lblMsg.Text = "Copying data From clipboard..."
        lblMsg.Refresh()
        Utils.ClipboardToDGV(uGrid.dgvData, True)
        lblMsg.Text = CStr(uGrid.dgvData.Rows.Count) & " rows are ready to upload."
        lblMsg.Refresh()

        btn5.Visible = True
    End Sub

Erroneous output

No event wiring. Function is converted correctly but event handler function is not wired to btn2.Click event

Expected output

Event handler function should be wired to btn2.Click event

Details

  • Product in use: VS extension
  • Version in use: 2.11.106.19330
  • Did you see it working in a previous version, which?
  • Any other relevant information to the issue, or your interest in contributing a fix.

Problem occurs on forms that inherit base form which has the definition of btn2
In the base form, vb.net code had a public Button btn2, but in the c# version it has a private Button _btn2 and a getter/setter called btn2

The designer code for child forms are referencing in some cases the private class member, e.g. _btn2, which causes an error since _btn2 is private, and in other cases the public member, e.g. btn3. The modifier issue is minor, the main problem is the wiring of events.

@plalonde4321 plalonde4321 added the VB -> C# Specific to VB -> C# conversion label Oct 6, 2021
@plalonde4321 plalonde4321 changed the title VB -> C#: _Add a short description_ VB -> C#: Missing event wiring Oct 6, 2021
@GrahamTheCoder
Copy link
Member

Thanks for the report. The version number you mentioned would be pretty ancient, so do check that, but I'm guessing that's just a mixup.

I think we already have some tests around events on fields from a base class but I'll try to create a repro when I get a chance, perhaps this weekend.
Could you post the declaration of btn2?

@plalonde4321
Copy link
Author

plalonde4321 commented Oct 6, 2021

You are right about the version number, it's a mixup, version number is 8.4.1.0

From the base form designer.cs code

private Button _btn2;

        public Button btn2
        {
            [MethodImpl(MethodImplOptions.Synchronized)]
            get
            {
                return _btn2;
            }

            [MethodImpl(MethodImplOptions.Synchronized)]
            set
            {
                if (_btn2 != null)
                {
                    _btn2.Click -= btn2_Click;
                }

                _btn2 = value;
                if (_btn2 != null)
                {
                    _btn2.Click += btn2_Click;
                }
            }
        }

Base form has an event handler (btn2_Click)
Child form also defines an event handler with same name, that's the one with the missing event wiring.

Thanks.

@plalonde4321
Copy link
Author

Here is the original VB.net declaration:
Public WithEvents btn2 As System.Windows.Forms.Button

@GrahamTheCoder
Copy link
Member

GrahamTheCoder commented Oct 7, 2021

Thanks. I've managed to reproduce something that at least sounds similar using this

VB input:

Imports System
Imports System.Windows.Forms
Imports Microsoft.VisualBasic.CompilerServices

Partial Class BaseForm
    Inherits Form
    Friend WithEvents BaseButton As Button
End Class

<DesignerGenerated>
Partial Class Form1
    Inherits BaseForm
    Private Sub InitializeComponent()
        Me.BaseButton = New Button()
        Me.Button1 = New Button()
    End Sub
    Friend WithEvents Button1 As Button
End Class

Partial Class Form1
    Private Sub MultiClickHandler(sender As Object, e As EventArgs) Handles Button1.Click,
                                                                            BaseButton.Click
    End Sub
End Class

Erroneous C# output:

using System;
using System.Runtime.CompilerServices;
using System.Windows.Forms;
using Microsoft.VisualBasic.CompilerServices; // Install-Package Microsoft.VisualBasic

internal partial class BaseForm : Form
{
    internal Button BaseButton;
}

[DesignerGenerated]
internal partial class Form1 : BaseForm
{
    public Form1()
    {
        InitializeComponent();
    }

    private void InitializeComponent()
    {
        this._BaseButton = new Button();
        _BaseButton.Click += new EventHandler(MultiClickHandler);
        _Button1 = new Button();
        _Button1.Click += new EventHandler(MultiClickHandler);
    }

    private Button _Button1;

    internal Button Button1
    {
        [MethodImpl(MethodImplOptions.Synchronized)]
        get
        {
            return _Button1;
        }

        [MethodImpl(MethodImplOptions.Synchronized)]
        set
        {
            if (_Button1 != null)
            {
                _Button1.Click -= MultiClickHandler;
            }

            _Button1 = value;
            if (_Button1 != null)
            {
                _Button1.Click += MultiClickHandler;
            }
        }
    }
}

internal partial class Form1
{
    private void MultiClickHandler(object sender, EventArgs e)
    {
    }
}

Expected CSharp Output:

using System;
using System.Runtime.CompilerServices;
using System.Windows.Forms;
using Microsoft.VisualBasic.CompilerServices; // Install-Package Microsoft.VisualBasic

internal partial class BaseForm : Form
{
    internal Button _BaseButton;

    internal virtual Button BaseButton
    {
        get
        {
            return _BaseButton;
        }
    }
}

[DesignerGenerated]
internal partial class Form1 : BaseForm
{
    public Form1()
    {
        InitializeComponent();
    }

    private void InitializeComponent()
    {
        _BaseButton = new Button();
        _BaseButton.Click += new EventHandler(MultiClickHandler);
        _Button1 = new Button();
        _Button1.Click += new EventHandler(MultiClickHandler);
    }

    internal override Button BaseButton
    {
        get
        {
            return base.BaseButton;
        }
        [MethodImpl(MethodImplOptions.Synchronized)]
        set
        {
            if (baseButton != null)
            {
                base.BaseButton.Click -= new Button.ClickEventHandler(MultiClickHandler);;
            }
            base.BaseButton = value;
            if (baseButton != null)
            {
                base.BaseButton.Click += new Button.ClickEventHandler(MultiClickHandler);;
            }
        }
    }

    private Button _Button1;

    internal Button Button1
    {
        [MethodImpl(MethodImplOptions.Synchronized)]
        get
        {
            return _Button1;
        }

        [MethodImpl(MethodImplOptions.Synchronized)]
        set
        {
            if (_Button1 != null)
            {
                _Button1.Click -= MultiClickHandler;
            }

            _Button1 = value;
            if (_Button1 != null)
            {
                _Button1.Click += MultiClickHandler;
            }
        }
    }
}

internal partial class Form1
{
    private void MultiClickHandler(object sender, EventArgs e)
    {
    }
}

i.e. Declare internal field and virtual propertyin the base and override it in the inheriting class

@plalonde4321
Copy link
Author

Yes that's the jist of it. The button defined in the base class no longer has its click event wired to the handler defined in the inheriting class. The "expected CSharp output" from your comment has the correct event wiring.

@plalonde4321
Copy link
Author

Any ETA on when this will be fixed? Thanks!

@GrahamTheCoder
Copy link
Member

The best I can say is "not in the next week". I'll try to find time to at least attempt it over Christmas, but may have to delay it if it's turning out too big. I'll bump it up my list a bit since it seems you're waiting for it, but there's another big issue above it - any particular time you're planning to do the conversion that it'd be handy to have this by?

@GrahamTheCoder GrahamTheCoder changed the title VB -> C#: Missing event wiring VB -> C#: Missing event wiring for overridden members Dec 12, 2021
@plalonde4321
Copy link
Author

Thanks! The sooner the better to do the conversion, trying to get rid of vb.net and have all C# for our apps but I understand there might be other priorities to tackle and it's not causing problems per say. If it could get done before end of Q1 2022 that would be great! This is a great tool by the way!

@GrahamTheCoder
Copy link
Member

I started spiking a fix for this recently. It's quite a tricky set of constraints but I'm reasonably confident in the solution. I'll try to complete it next week but can't promise anything and it'll still have some edge cases that break I'm sure

@GrahamTheCoder
Copy link
Member

I've made some decent progress on the small test case now. I'll try it on some real apps tomorrow hopefully and refine it. If you're willing to try out a prerelease build from the build system (or build it locally and try it), let me know and I can give some hints on doing so if it's not obvious (in particular something a bit odd has happened with the local debug options attaching a debugger when building from source, so it's best to just build it then install the output and restart)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VB -> C# Specific to VB -> C# conversion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants