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

Proposal: Option to omit type for accessors when type is known or can be inferred #417

Closed
ReinBentdal opened this issue Jun 22, 2019 · 10 comments
Labels
state-duplicate This issue or pull request already exists

Comments

@ReinBentdal
Copy link

ReinBentdal commented Jun 22, 2019

What i would like to do (example in flutter):

void weight(FontWeight fontWeight) {
  // do something
}

weight(.bold);

Since a FontWeight is expected, there should be an option to omitt that part and only write the method inside the FontWeight class.
The same goes for enums.

enum MyEnum {one, two, three}

void number(MyEnum myEnum) {
  // do something
}
number(.two); // instead of number(MyEnum.two)
@andrewackerman
Copy link

This is how Swift handles enum literals and const/static values. If a location is expecting a value of a certain type (such as a variable or a parameter where the type is known at compile time), the type identifier can be omitted as it can easily be inferred.

That being said, I'm sure there's going to be syntax ambiguities that would prohibit this from being so simple to implement. One example that comes to mind is using this feature in combination with a ternary operator:

fontWeight: isBold ? .bold : .normal

The ? .bold bit could be confusing for the compiler as ?. is the null-aware accessor operator. This would be especially a problem for people who like to omit spaces before and after their operators, making the ternary look like isBold?.bold:.normal which makes it completely ambiguous whether the ?. is a ternary followed by a type-inferred static accessor of FontWeight.bold or if it's a null-aware accessor trying to get the value of isBold.bold.

At any rate, this would be a cool feature, but IMO you need to change the title of the issue. It's not clear at all what your proposal is based on the title alone, so it's not going to see much long-term traffic.

@ReinBentdal ReinBentdal changed the title Proposal: Option to omitt excpected code Proposal: Omitt expression when a certain expression is expected. expression.memberName -> .memberName Jun 22, 2019
@ReinBentdal
Copy link
Author

ReinBentdal commented Jun 22, 2019

I can see how that could be a problem. Do you know how Swift handles that problem?
Isnt the spaces enough for the compiler to know how to treat it, or doesnt the space matter?

@andrewackerman
Copy link

I don't know enough about the exact semantics of either Dart or Swift to answer that. We'll have to wait for a member of the Dart team to comment.

If I had to guess, I would say that Swift is much more reliant on whitespace than Dart, so it can do things like mandate there be spaces around operators at the language level. For Dart, omitting the spaces around operators, while discouraged (and usually a linter error), is still valid Dart code, so even if the compiler could use the space between the ? and the . to resolve the ambiguity, it still needs to be able to know how to deal with a situation when the space isn't there.

P.S. The new title is wordier but still pretty vague. My suggestion for the title would be something along the lines of "Option to omit type for accessors when type is known or can be inferred".

@ReinBentdal ReinBentdal changed the title Proposal: Omitt expression when a certain expression is expected. expression.memberName -> .memberName Proposal: Option to omit type for accessors when type is known or can be inferred Jun 22, 2019
@ReinBentdal
Copy link
Author

Another example on how this could change code written in flutter
(This is just a dummie code and doesnt make sense)

Before

Column(
  mainAxisAlignment: MainAxisAlignment.spaceBetween,
  mainAxisSize: MainAxisSize.min,
  children: <Widget>[
    Text(
      'some text',
      textAlign: TextAlign.center,
      overflow: TextOverflow.visible,
      textDirection: TextDirection.rtl,
    ),
    Align(
      alignment: Alignment.center,
      child: Padding(
        padding: EdgeInsets.only(top: 10),
        child: Text('some more text'),
      ),
    )
  ],
);

After

Column(
  mainAxisAlignment: .spaceBetween,
  mainAxisSize: .min,
  children: <Widget>[
    Text(
      'some text',
      textAlign: .center,
      overflow: .visible,
      textDirection: .rtl,
    ),
    Align(
      alignment: .center,
      child: Padding(
        padding: .only(top: 10),
        child: Text('some more text'),
      ),
    )
  ],
);

In my opinion it makes for much cleaner and more sensible code. Gone are all the repetitions.

@frankzang
Copy link

frankzang commented Jun 23, 2019

I think this is tightly related to the issue #357

@ReinBentdal
Copy link
Author

There may be some issues where for example a AlignmentGeometry is expected

Align(
  alignment: .center
)

When the expected type has a different name than the value given there may be some issues:

  • Multiple classes extended from the AlignmentGeometry with a method with the same name.
    FractionalOffset.center and Alignment.center

This feature request may only work if the expected type is the exact same as the type given.

Not possible

align: .center // expects [AlignmentGeometry], recieves [Alignment]
margin: .only(left: 10) // expects [EdgeInsetsGeometry], recieves [EdgeInsets]

Possible

textAlign: .center // expects [TextAlign], recieves [TextAlign]
color: .black // expects [Color], recieves [Color]

I gues the only solution from here is for flutter to adapt to this restraint, but that isnt very likely.

@andrewackerman
Copy link

Pretty much the only way this feature would work is if the type inference will expect a value of the exact type specified by the context. For the example of Padding, because the padding property is defined as being an EdgeInsetsGeometry, the inference will look at that type for static constants or methods of the given name. There's no way we could make it work for any arbitrary subclass or utility class, or at least making that work will add so much complexity that it becomes impractical.

In the future, this could be resolved with static extension methods. Doing that, it would be possible to add a static only method to EdgeInsetsGeometry, for example, that just wraps a call to EdgeInsets.only. (It's either this or change the Flutter SDK so that utility classes like EdgeInsets and Alignment become the wrappers and the actual factory code gets moved to methods in the actual class.)

@andrewackerman
Copy link

Another thing that occurred to me is that this feature could also be used for named/factory constructors:

List<int> numbers = .generate(5, (i) => i);

A bit of a contrived example (and less useful as you could just use var), but hopefully it conveys the concept.

@lrhn
Copy link
Member

lrhn commented Jun 24, 2019

This is a duplicate of #357. Let's continue the discussion there.

@lrhn lrhn closed this as completed Jun 24, 2019
@lrhn lrhn added the state-duplicate This issue or pull request already exists label Jun 24, 2019
@andrewackerman
Copy link

@lrhn Is there a way to transfer the reactions over as well? This issue got 4 times as many public reactions as that thread did and broke the top 10 in reacted-to issues within a day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state-duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants