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

Missing an option between "Prefer var for built-in types" and "when variable type is apparent" #23714

Open
davkean opened this issue Dec 11, 2017 · 7 comments
Labels
Area-IDE Feature Request Need Design Review The end user experience design needs to be reviewed and approved. Resolution-By Design The behavior reported in the issue matches the current design
Milestone

Comments

@davkean
Copy link
Member

davkean commented Dec 11, 2017

There doesn't seem a combination of var options that allows the following:

            var boolValue = true;
            var stringValue = "Hello World!";
            var longValue = 0L;

but disallows the following:

            var boolValue = GetSomeValue();
            var stringValue = GetAnotherValue();
            var longValue = GetYetAnotherValue();

In my opinion, the former should fall under "when variable type is apparent".

@sharwell
Copy link
Member

This is by design. The rules are applied in the following order:

  1. Preference for built-in types
  2. Preference for cases where the variable type is apparent
  3. Preference for other cases

As soon as an applicable case is encountered from this list, the remaining items are ignored.

@sharwell sharwell added the Resolution-By Design The behavior reported in the issue matches the current design label Dec 11, 2017
@davkean
Copy link
Member Author

davkean commented Dec 11, 2017

I'm asking for this behavior to be revisited

@davkean davkean reopened this Dec 11, 2017
@davkean
Copy link
Member Author

davkean commented Dec 11, 2017

I don't care how this is designed, but it is odd to me that with in-built types you can't have a "use var when type is apparent" behavior.

@jcouv jcouv added Area-IDE Feature Request and removed Resolution-By Design The behavior reported in the issue matches the current design labels Dec 12, 2017
@jinujoseph jinujoseph added this to the Unknown milestone Jan 8, 2018
@sharwell sharwell added Need Design Review The end user experience design needs to be reviewed and approved. and removed Discussion labels May 1, 2018
@sharwell sharwell modified the milestones: Unknown, 15.8 May 1, 2018
@jinujoseph jinujoseph modified the milestones: 15.8, Unknown May 29, 2018
@sharwell sharwell added this to In Queue in IDE: Design review Jun 18, 2018
@sharwell sharwell moved this from In Queue to Next meeting (opportunistic) in IDE: Design review Jun 18, 2018
@sharwell sharwell added the Resolution-By Design The behavior reported in the issue matches the current design label Jun 18, 2018
@sharwell sharwell moved this from Next meeting (opportunistic) to Next meeting (priority) in IDE: Design review Jun 18, 2018
@sharwell
Copy link
Member

Tentatively marked By Design since the current behavior was not unintentional; will update with discussion outcome following design review.

@CyrusNajmabadi
Copy link
Member

@sharwell Note that you mentioned this will go to the next design meeteing and will be updated with the results. However, the results (if this happened) were not posted to the issue. can you update accordingly? Thanks!

@osdm
Copy link

osdm commented Oct 19, 2020

@sharwell Could you please share a logic behind current design? I cannot really imagine any case where user would want to use var for apparent types except for apparent built-in types. That means logic like this:
useVar = IsBuildInType && PreferVarForBuildInTypes || IsApparent && PreferVarForApparentTypes || !IsBuiltInType && !IsApparent && PreferVarElsewhere;
Is there anything wrong with this approach?

@sharwell
Copy link
Member

sharwell commented Oct 19, 2020

I cannot really imagine any case where user would want to use var for apparent types except for apparent built-in types.

It's common for StyleCop users. Even if we ended up with people generally agreeing that either order works, the types in question here are very common and any change would require many teams to make large numbers of changes to their code to fix the new warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Feature Request Need Design Review The end user experience design needs to be reviewed and approved. Resolution-By Design The behavior reported in the issue matches the current design
Projects
Status: Need Update
IDE: Design review
  
Need Update
Development

No branches or pull requests

6 participants