From 90fd1a8d08319df194873d3a359d89b679ce421b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 1 Feb 2017 00:09:59 -0500 Subject: [PATCH] `override' implies `virtual' This behavior is consistent with other OO languages like C++ and C# that do not have virtual methods by default. This solution isn't ideal, but I don't have time for a larger refactoring right now. I sat on this change for a good few weeks before committing it unchanged. * lib/MemberBuilderValidator.js (validateMethod): Allow override of supertype overrides. * test/*: Stripped `virtual' keyword where appropriate. * doc/classes.texi (Inheritance): Update to state that `override' implies `virtual'. --- doc/classes.texi | 24 +++++++++++++++++------ lib/MemberBuilderValidator.js | 3 ++- test/MemberBuilderValidator/MethodTest.js | 15 +++++++++++--- test/Trait/ClassExtendTest.js | 10 +++++----- test/Trait/LinearizationTest.js | 4 ++-- test/Trait/VirtualTest.js | 2 +- test/perf/perf-trait-mixin.js | 6 +++--- 7 files changed, 43 insertions(+), 21 deletions(-) diff --git a/doc/classes.texi b/doc/classes.texi index 25f8db3..25fb45c 100644 --- a/doc/classes.texi +++ b/doc/classes.texi @@ -749,12 +749,10 @@ One of the following conditions must always be true: @var{dfn\_n\^C} is declared with the @ref{Member Keywords,,@code{virtual}} keyword and @var{dfn\_n\^C'} is declared with the @ref{Member Keywords,,@code{override}} keyword. - @itemize - @item - Note that @var{dfn\_n\^C'} will not become @ref{Member Keywords,,@code{virtual}} - by default (unlike languages such as C++); they must be explicitly - declared as such. - @end itemize + @item + @var{dfn\_n\^C} is declared with the @ref{Member Keywords,,@code{override}} + keyword and @var{dfn\_n\^C'} is also declared with the + @ref{Member Keywords,,@code{override}} keyword. @item @var{dfn\_n\^C} is declared with the @ref{Member Keywords,,@code{abstract}} keyword and @var{dfn\_n\^C'} omits the @ref{Member Keywords,,@code{override}} @@ -860,10 +858,24 @@ classes using ease.js? console.log( 'Walking the dog on two feet' ); } } ); + + // supertype override is implicitly virtual + var ReallyLazyDog = LazyDog.extend( + { + 'override public walk': function() + { + // ... + } + } ); @end verbatim @caption{Inheritance in ease.js} @end float +(The above inheritance tree is a bad idea and is for illustration + purposes only: + if you want to layer attributes, + use Traits or a composition-based pattern like Decorators.) + You should already understand how to define a class (@pxref{Defining Classes}). The above example introduced two means of @dfn{extending} classes -- defining a new class that inherits from a parent: diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index 85c2e30..fe20dc7 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -211,7 +211,8 @@ exports.prototype.validateMethod = function( } // disallow overriding non-virtual methods - if ( keywords[ 'override' ] && !( prev_keywords[ 'virtual' ] ) ) + if ( keywords[ 'override' ] + && !( prev_keywords[ 'virtual' ] || prev_keywords[ 'override' ] ) ) { if ( !( keywords[ 'abstract' ] ) ) { diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index c2c42ef..230755a 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -192,9 +192,18 @@ require( 'common' ).testCase( /** - * Overriding a method in ease.js does not immediately make it virtual. - * Rather, the virtual keyword must be explicitly specified. Let's ensure - * that it is permitted. + * An override is implicitly virtual. This was not always the case: + * prior to v0.2.9, an override did not imply virtual. + */ + 'Can override overridden method with concrete method': function() + { + this.quickKeywordMethodTest( [ 'override' ], null, [ 'override' ] ); + }, + + + /** + * For legacy reasons; see above test. In the future, this might + * trigger a notice, since it's redundant. No harm in being explicit! */ 'Can declare override as virtual': function() { diff --git a/test/Trait/ClassExtendTest.js b/test/Trait/ClassExtendTest.js index b538449..7e64104 100644 --- a/test/Trait/ClassExtendTest.js +++ b/test/Trait/ClassExtendTest.js @@ -485,7 +485,7 @@ require( 'common' ).testCase( var T1 = this.Sut.extend( C, { - 'virtual abstract override foo': function() + 'abstract override foo': function() { return 3 + this.__super(); }, @@ -493,7 +493,7 @@ require( 'common' ).testCase( var T2 = this.Sut.extend( C, { - 'virtual abstract override foo': function() + 'abstract override foo': function() { return 13 + this.__super(); }, @@ -533,7 +533,7 @@ require( 'common' ).testCase( var T1 = this.Sut.extend( C, { - 'virtual override foo': function() + 'override foo': function() { return 3 + this.__super(); }, @@ -541,7 +541,7 @@ require( 'common' ).testCase( var T2 = this.Sut.extend( C, { - 'virtual override foo': function() + 'override foo': function() { return 13 + this.__super(); }, @@ -603,7 +603,7 @@ require( 'common' ).testCase( var T2 = this.Sut.extend( C, { // T1 provides a concrete method that we can override - 'virtual abstract override concrete': function() + 'abstract override concrete': function() { return 5 + this.__super(); }, diff --git a/test/Trait/LinearizationTest.js b/test/Trait/LinearizationTest.js index c21e394..e3d2535 100644 --- a/test/Trait/LinearizationTest.js +++ b/test/Trait/LinearizationTest.js @@ -137,7 +137,7 @@ require( 'common' ).testCase( var Ta = this.Sut.implement( I ).extend( { - 'virtual abstract override foo': function() + 'abstract override foo': function() { called.a = true; this.__super(); @@ -182,7 +182,7 @@ require( 'common' ).testCase( var T = this.Sut.implement( I ).extend( { - 'virtual abstract override foo': function() + 'abstract override foo': function() { called++; this.__super(); diff --git a/test/Trait/VirtualTest.js b/test/Trait/VirtualTest.js index 4dcf068..dc9939b 100644 --- a/test/Trait/VirtualTest.js +++ b/test/Trait/VirtualTest.js @@ -340,7 +340,7 @@ require( 'common' ).testCase( this.assertStrictEqual( expected, this.Class.use( T ).extend( { - 'override virtual foo': function() + 'override foo': function() { return this.__super(); }, diff --git a/test/perf/perf-trait-mixin.js b/test/perf/perf-trait-mixin.js index a2ea59c..8042361 100644 --- a/test/perf/perf-trait-mixin.js +++ b/test/perf/perf-trait-mixin.js @@ -55,9 +55,9 @@ var Cv = Class.implement( I ).extend( var To = Trait.implement( I ).extend( { - 'virtual abstract override a': function() {}, - 'virtual abstract override b': function() {}, - 'virtual abstract override c': function() {}, + 'abstract override a': function() {}, + 'abstract override b': function() {}, + 'abstract override c': function() {}, } );