Skip to content

Commit

Permalink
Removed @OverRide (#256)
Browse files Browse the repository at this point in the history
  • Loading branch information
mlhaufe authored Aug 27, 2023
1 parent dffb1cb commit fed97fd
Show file tree
Hide file tree
Showing 17 changed files with 48 additions and 753 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Project targets ES2022 to utilize the [new language feature](https://devblogs.microsoft.com/typescript/announcing-typescript-5-2/#decorator-metadata)
* Converted project to utilize ESM
* Simplified eslint configuration
* Removed `@override` decorator in favor of [native](https://devblogs.microsoft.com/typescript/announcing-typescript-4-3-beta/#override-and-the-noimplicitoverride-flag) `override` keyword and `@override` [JSDoc tag](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#override)

## v0.24.2

Expand Down
105 changes: 5 additions & 100 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
- [Assertions](#assertions)
- [Implies](#implies)
- [Iff](#iff)
- [Overrides](#overrides)
- [Encapsulation](#encapsulation)
- [Invariants](#invariants)
- [Demands](#demands)
Expand Down Expand Up @@ -298,96 +297,6 @@ iff(

This is logically equivalent to: `implies(p,q) && implies(q,p)`

## Overrides

Class features implemented in a base class can be overridden in a subclass. The
subclass implementation can augment or entirely replace the one belonging
to the base class. This can be done for a variety of reasons such as
providing a more efficient implementation in the context of the subclass.
Regardless of the reason the overridden member should be semantically
consistent with the base class member. In other
words it should follow [Liskov's Substitution Principle](https://en.wikipedia.org/wiki/Liskov_substitution_principle).
To aid in the enforcement and documentation of this principle the library
provides an `@override` decorator for class methods and accessors.

A simple example is calculating the area of Convex Polygons. While a [general
formula](https://en.wikipedia.org/wiki/Heron%27s_formula) exists to accomplish this
more efficient and direct formulas exist for specific polygons such as a Right Triangle:

```typescript
type Side = number
type Vertex = [number, number]

function _triArea(v1: Vertex, v2: Vertex, v3: Vertex): number {
const {hypot,sqrt} = Math,
a = hypot((v1[0] - v2[0]), (v1[1] - v2[1])),
b = hypot((v2[0] - v3[0]), (v2[1] - v3[1])),
c = hypot((v3[0] - v1[0]), (v3[1] - v1[1])),
s = 0.5 * (a + b + c)

return sqrt(s*(s-a)*(s-b)*(s-c))
}

@Contracted()
class ConvexShape {
#vertices: Vertex[]

constructor(...vertices: Vertex[]) {
this.#vertices = vertices
}

area(): number {
let [v1, v2, v3, ...vs] = this.#vertices
return this.#vertices.length >= 3 ?
_triArea(v1, v2, v3) + new ConvexShape(v1, v3, ...vs).area() :
0
}

get vertices(): Vertex[] { return this.#vertices.slice() }
}

class RightTriangle extends ConvexShape {
#base: Side
#height: Side

constructor(base: Side, height: Side) {
super([0,0], [base,0], [base,height])
this.#base = base
this.#height = height
}

@override
area(): number { return this.#base * this.#height / 2 }

get base(): Side { return this.#base }
get height(): Side { return this.#height }
}
```

Above you can see the `area()` method being overridden with the more
efficient implementation. The `@override` decorator makes explicit
that the method is replacing another implementation.

This decorator does not only document and verify that the method is
overridden; it will also verify that the parameter count matches.

Note that the current class or one of its ancestors must have `@Contracted(...)` assigned.
When assigned, candidate overrides are identified and an
error is raised if an associated `@override` is missing for that feature.

Static methods including the constructor cannot be assigned an `@override`
decorator. In the future this may be enabled for non-constructor static methods
but the implications are not clear at present.

[TypeScript 4.3](https://devblogs.microsoft.com/typescript/announcing-typescript-4-3-beta/#override-and-the-noimplicitoverride-flag) has added an `override` keyword.
This should be compatible but possibly redundant. The pro/cons are:

| `override` | `@override` |
|---------------------------|-------------------------------------|
| Compile Time | Runtime |
| TypeScript Only | TypeScript and JavaScript |
| Enforces type constraints | Enforces parameter count constraint |

## Encapsulation

To guarantee invariants remain valid for classes, public property definitions are forbidden.
Expand Down Expand Up @@ -526,8 +435,7 @@ If a class feature is overridden then the `demands` assertion still applies:

```typescript
class MyStack<T> extends Stack<T> {
@overrides
pop(): { ... }
override pop(): { ... }
}

...
Expand Down Expand Up @@ -579,8 +487,7 @@ const subContract = new Contract<Sub>({

@Contracted(subContract)
class Sub extends Base {
@override
someMethod(x: number){ ... }
override someMethod(x: number){ ... }
}
```

Expand Down Expand Up @@ -642,8 +549,7 @@ If a class feature is overridden then the `ensures` assertion still applies:

```typescript
class MyStack<T> extends Stack<T> {
@overrides
push(value: T): { ... }
override push(value: T): { ... }
}

...
Expand Down Expand Up @@ -829,8 +735,7 @@ class Base {
}

class Sub extends Base {
@override
myMethod(){ ... throw new Error('BOOM!') ... }
override myMethod(){ ... throw new Error('BOOM!') ... }
}
```

Expand All @@ -839,7 +744,7 @@ Another capability that the `rescue` declaration provides is
to enable [Fault-Tolerance](https://en.wikipedia.org/wiki/Fault_tolerance)
and [Redundancy](https://en.wikipedia.org/wiki/Redundancy_(engineering)).

A dated example of this is performing ajax requests in mult-browser environments where `fetch` may not exist:
A dated example of this is performing ajax requests in multi-browser environments where `fetch` may not exist:

```ts
const ajaxRequestContract = new Contract<AjaxRequest>({
Expand Down
15 changes: 1 addition & 14 deletions TODO.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
Prevent delete

============

@override needs to test input for use in non-TS environments. assert(isFunction) and such

does an @override member return a value as expected?

==========
async method handling?
<https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch>

=============
overrides
writable | configurable | enumerable verification

Does the typescript 4.3 override feature not cover these use cases? Therefore does not subsume this
library feature?

=========
<https://web.archive.org/web/20151205005155/https://blog.mozilla.org/dherman/2011/08/29/contracts-coffee/>
<https://web.archive.org/web/20151002033716/http://c2.com/cgi/wiki?DesignByContract>
Expand Down Expand Up @@ -56,7 +43,7 @@ Unit testing is one thing, but a test plan is needed as well to show that the li
Stack vs Queue interface

========

competitor/comparable
<https://github.com/alexreardon/tiny-invariantcompare> with other contract libraries
<https://en.wikipedia.org/wiki/Design_by_contract>
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"LSP",
"material-implication",
"organized-panic",
"overrides",
"precondition",
"postcondition",
"rescue",
Expand Down Expand Up @@ -67,4 +66,4 @@
"files": [
"dist"
]
}
}
11 changes: 1 addition & 10 deletions src/Contracted.mts
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,8 @@ function Contracted<
assert(!hasProperties(classRegistration, this), MSG_NO_PROPERTIES);

if (!classRegistration.contractsChecked) {
let ancRegistrations = classRegistration.ancestry().reverse();
// top-down check overrides and pre-existing properties
[...ancRegistrations, classRegistration].forEach(ancRegistration => {
if (!ancRegistration.contractsChecked) {
ancRegistration.checkOverrides();
ancRegistration.contractsChecked = true;
}
});

// bottom-up to closest Contracted class bind contracts
ancRegistrations = takeWhile(ancRegistrations.reverse(), (cr => cr.Class !== Base));
const ancRegistrations = takeWhile(classRegistration.ancestry(), (cr => cr.Class !== Base));
[classRegistration, ...ancRegistrations, CLASS_REGISTRY.get(Base)!].forEach(registration => {
registration.bindContract(InnerContracted[innerContract]);
});
Expand Down
3 changes: 0 additions & 3 deletions src/Messages.mts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
*/

export const ASSERTION_FAILED = 'Assertion failure';
export const MSG_INVALID_ARG_LENGTH = 'An overridden method must have the same number of parameters as its ancestor method';
export const MSG_NO_STATIC = 'Only instance members can be decorated, not static members';
export const MSG_NO_MATCHING_FEATURE = 'This feature does not override an ancestor feature.';
export const MSG_DUPLICATE_OVERRIDE = 'Only a single @override decorator can be assigned to a class member';
export const MSG_NO_PROPERTIES = 'Public properties are forbidden';
export const MSG_NOT_CONTRACTED = 'The current class or one of its ancestors must declare @Contracted(...)';
export const MSG_MISSING_FEATURE = 'The requested feature is not registered';
Expand Down
3 changes: 1 addition & 2 deletions src/index.mts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@

import AssertionError from './AssertionError.mjs';
import Contracted, { innerContract } from './Contracted.mjs';
import override from './override.mjs';
import assert from './assert.mjs';
import implies from './implies.mjs';

export { AssertionError, Contracted, implies, innerContract, override, assert };
export { AssertionError, Contracted, implies, innerContract, assert };
export { Contract, extend, invariant, Invariant, checkedMode, Rescue } from './Contract.mjs';
export { Constructor, AbstractConstructor, ClassType } from './lib/ClassType.mjs';
13 changes: 0 additions & 13 deletions src/lib/ClassRegistration.mts
Original file line number Diff line number Diff line change
Expand Up @@ -166,19 +166,6 @@ class ClassRegistration {
});
}

/**
* Checks the features of the registered class for missing override decorators
* @throws {AssertionError} - Throws if the verification fails
*/
checkOverrides(): void {
const ancestryFeatureNames = new Set(this.ancestryFeatures().map(({ name }) => name));
this.features.forEach(({ name, hasOverrides }) => {
const str = `${this.Class.name}.prototype.${String(name)}`;
assert(!hasOverrides || ancestryFeatureNames.has(name), `Unnecessary @override declaration on ${str}`);
assert(hasOverrides || !ancestryFeatureNames.has(name), `@override decorator missing on ${str}`);
});
}

/**
* Searches the current class and its ancestors for the nearest feature
* matching the provided propertyKey.
Expand Down
16 changes: 0 additions & 16 deletions src/lib/Feature.mts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@ import ClassRegistration from './ClassRegistration.mjs';

class Feature {
#descriptor: PropertyDescriptor;
#overriddenOriginalDescriptor: PropertyDescriptor | undefined;

/**
* Does the current feature have an `@override` declaration?
*/
hasOverrides = false;

constructor(readonly classRegistration: ClassRegistration, readonly name: PropertyKey, descriptor: PropertyDescriptor) {
this.#descriptor = descriptor;
Expand Down Expand Up @@ -96,16 +90,6 @@ class Feature {
return this.#descriptor.value;
}

/**
* The original feature descriptor which was replaced by the `override` decorator
*/
get overriddenOriginalDescriptor(): PropertyDescriptor | undefined {
return this.#overriddenOriginalDescriptor;
}
set overriddenOriginalDescriptor(value: PropertyDescriptor | undefined) {
this.#overriddenOriginalDescriptor = value;
}

/**
* Returns a reference to the descriptor
*/
Expand Down
61 changes: 0 additions & 61 deletions src/override.mts

This file was deleted.

2 changes: 0 additions & 2 deletions src/tests/Contract.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ interface StackType<T> {

// https://github.com/final-hill/decorator-contracts/issues/171
describe('A contract must be independently definable', () => {

test('Well typed contract', () => {
const stackContract = new Contract<StackType<any>>({
pop: {},
Expand Down Expand Up @@ -204,6 +203,5 @@ describe('A subclass can only be contracted by a subcontract of the base class c
@Contracted(badContract)
class Bar extends Foo { }
}).toThrow(MSG_BAD_SUBCONTRACT);

});
});
Loading

0 comments on commit fed97fd

Please sign in to comment.