dart:core: replace @foo with @Foo() #14860
Comments
|
Added Area-Library, Library-Core, Triaged labels. |
|
CCing people that have more opinions on annotations. cc @lrhn. |
|
I agree completely. I think the proposed approach would be much better. (And it would be kind of cute to see 'deprecated' be marked as being deprecated.) |
|
This comment was originally written by @seaneagan Looks like we ran out of time on this one. IMO, there should still be a style guide rule (issue #4) to at least PREFER @Foo() over @foo, and this should be done quickly before a bunch of important packages which expose annotations start reaching 1.0 status. It could be seen as quite similar to num/int/double/bool violating the UpperCamelCase class names style guide rule. I filed issue #9 specifically for polymer/observe annotations. |
|
I like the idea in 13582 that you can omit '()' as it will make it easier to transition between no-arg and some args on some annotations. However, I personally prefer @foo over @Foo(). I understand this might be a matter of taste, but maybe we should try to agree on what we want the styleguide for annotations to be. I'd be more inclined to have a different style for annotations that makes them especially readable. For example, if a class is intended for annotations, maybe we want to use @lowerCamelCase or @lower_case_with_underscores rather than @UpperCamelCase. Just my 2 cents. |
|
Removed Type-Defect label. |
DartBot
added
Type-Enhancement
area-core-library
kevmoo
added
type-enhancement
This issue was originally filed by @seaneagan
Currently in dart:core we have top-level variables for annotations:
@deprecated
@Proxy
@OverRide
But in order to allow specifying an expiration date, the Deprecated class is also public:
@deprecated('1/1/2014')
This means when I refactor between the 2 forms, I have to convert between 'd' and 'D', which is annoying. If the argument to the Deprecated constructor were made optional, then
deprecatedcould be removed, since you could just do:@deprecated()
If we ever need to add an optional argument to @proxy, we would similarly need to add a Proxy class, but we couldn't add it to dart:core, because it would break existing apps, and putting it in some other library away from @proxy would be really strange. For example, maybe we would want to limit the interfaces implemented by the proxy, to those explicitly stated in the implements clause:
@Proxy(explicitInterfaces: true)
class Foo implements A, B {
// ...
noSuchMethod(Invocation invocation) {
}
}
... so that
new Foo().someBogusFieldstill yields warnings:Also, if this pattern of @foo + @Foo(bar) is introduced in dart:core, it will set a style precedent to be followed by other annotations. This will lead to a lot of unnecessary top-level variables, which lead to name conflicts, shadowing issues, and unnecessary API surface.
Suggestion is to instead use:
@deprecated()
@Proxy()
@OverRide()
And now with issue #7 fixed, there is the possibility of issue #2 getting fixed in the future, at which point these could be shortened to:
@deprecated
@Proxy
@OverRide
The text was updated successfully, but these errors were encountered: