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

Suggestion: VB.NET lightbulb for removing setter bulk code #31101

Open
hartmair opened this issue Nov 10, 2018 · 17 comments
Open

Suggestion: VB.NET lightbulb for removing setter bulk code #31101

hartmair opened this issue Nov 10, 2018 · 17 comments
Labels
Area-IDE Feature Request Language-VB Need Design Review The end user experience design needs to be reviewed and approved.
Milestone

Comments

@hartmair
Copy link
Contributor

It would be nice to have a lightbulb quickaction for removing redundant setter bulk code (value as …). Originated from dotnet/vblang#344 (comment)

Version Used:
15.8.9
15.9.0 Preview 5.0

Steps to Reproduce:

Class Class1
    Property Property1 As Integer
        Get
            ...
        End Get
        Set(value As Integer)
            ' do something with value
        End Set
    End Property
End Class

Expected Behavior:
Lightbulb QuickAction to remove redundant setter bulk code (value as ...)

Class Class1
    Property Property1 As Integer
        Get
            ...
        End Get
' lightbulb on next line which changes into
        Set
            ' do something with value
        End Set
    End Property
End Class

Actual Behavior:
None

@CyrusNajmabadi
Copy link
Member

@hartmair Would you be interested in contributing such a fix?

@Neme12
Copy link
Contributor

Neme12 commented Nov 10, 2018

Interesting... would this fall into "code style" or "code quality"? I can imagine some people preferring to always be explicit here.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Nov 10, 2018

I would count it as 'style' as it's effectively a decision about implicit/explicit naming here. It feels like it would fit naturally into our 'fade out if necessary and can be trivially removed' bucket.

@CyrusNajmabadi
Copy link
Member

@Neme12 Would you like to take this one?

@Neme12
Copy link
Contributor

Neme12 commented Nov 10, 2018

I'll add this to my backlog, but VB is not something I am super excited about 😄

@CyrusNajmabadi
Copy link
Member

fair enough :)

@hartmair
Copy link
Contributor Author

@CyrusNajmabadi

@hartmair Would you be interested in contributing such a fix?

Yes, however, I would need a lot of guidance since I don't know anything of Roslyn yet.

@Neme12

I'll add this to my Backlog

thx. I am glad to help if it is useful to you.

@jinujoseph jinujoseph added help wanted The issue is "up for grabs" - add a comment if you are interested in working on it Language-VB Feature Request Need Design Review The end user experience design needs to be reviewed and approved. labels Nov 13, 2018
@jinujoseph jinujoseph added this to the Unknown milestone Nov 13, 2018
@jinujoseph jinujoseph removed the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Nov 13, 2018
@jinujoseph jinujoseph added this to In Queue in IDE: Design review via automation Nov 13, 2018
@jrmoreno1
Copy link
Contributor

Would this be done with an Analyzer/CodeFixProvider?

@CyrusNajmabadi
Copy link
Member

@jrmoreno1 Yes it would.

@jrmoreno1
Copy link
Contributor

jrmoreno1 commented Feb 1, 2019

I'd be willing to take it on as well, if @Neme12 doesn't mind.

The diagnostic and code fix in themselves aren’t a problem, but I’m not sure just where in the Roslyn source tree they should go.

roslyn/src/Features/VisualBasic/Portable?

With a name like RemoveUnecessarySetParameter?

@jrmoreno1
Copy link
Contributor

Should it be in VB or C#?
dotnet/vblang#360

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Feb 5, 2019

@jrmoreno1 This should be written in VB.

Here are the rules for IDE features:

  1. For a feature that is shared between C# and VB, we write the common code for it in C# in a shared lib we call 'Features'. Any C# or VB-specific pieces of hte feature are then in teh 'CSharpFeatures' and 'VisualBasicFeatures' projects.
  2. For any features that are totally language specific (i.e. "using pattern matching" in C#, or this new feature you're discussing), we'd place all the code in 'CSharpFeatures' or 'VisualBasicFeatures'.

@jrmoreno1
Copy link
Contributor

@CyrusNajmabadi : what version of the framework?

@CyrusNajmabadi
Copy link
Member

@jrmoreno1 i don't understand your question. can you clarify it?

@jrmoreno1
Copy link
Contributor

jrmoreno1 commented Feb 5, 2019

Perhaps it’s a silly question. I was first going to create a project outside of the Roslyn solution and wanted to know what framework version to use, but on thinking about it that probably won’t mattter when I figure out where to put it. I’ll try to get further on my own before bothering you again.

Also, I somehow missed your comments on where it should go on first reading, thanks for the additional info.

@CyrusNajmabadi
Copy link
Member

@jrmoreno1 Ah gotcha.

And don't worry about bothering me. I'm happy to help. Note: it may be faster to use gitter.im for that purpose though.

@jrmoreno1
Copy link
Contributor

jrmoreno1 commented Feb 8, 2019

@hartmair, @CyrusNajmabadi :
Would this be expected to operate or not on code decorated with generated code attributes?

GeneratedCodeAttribute, CompilerGeneratedAttribute, DesignerGenerated, SuppressMessageAttribute, others? If checking for this, do so at the class or property level?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Feature Request Language-VB Need Design Review The end user experience design needs to be reviewed and approved.
Projects
Status: In Queue
IDE: Design review
  
In Queue
Development

No branches or pull requests

6 participants