From 59fda51970b686fd25f59d12f6e51c2f7c1a942d Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Mon, 8 Apr 2019 12:33:34 -0700 Subject: [PATCH 1/5] feat: integrate flow type-checker This change adds flow comment syntax & runs flow check on lint that will fail on type missmatches. Additionally more type-friendly `CID.matchCID` alternative to `CID.isCID` is added which returns CID if value is CID or void otherwise. Method `.equals` is updated so parametre can by an arbirtary value, not just CID. It makes working with CIDs & type-checker a lot easier as it avoids refining `v` to `CID` type before being able to call `cid.equals(v)`. --- .prettierignore | 2 + package.json | 3 +- src/cid-util.js | 6 ++- src/index.js | 107 +++++++++++++++++++++++++++++++++++---------- src/index.js.flow | 33 -------------- test/index.spec.js | 32 ++++++++++++++ 6 files changed, 125 insertions(+), 58 deletions(-) create mode 100644 .prettierignore delete mode 100644 src/index.js.flow diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 0000000..7988935 --- /dev/null +++ b/.prettierignore @@ -0,0 +1,2 @@ +**/*.js +!**/*.flow.js \ No newline at end of file diff --git a/package.json b/package.json index b526151..0fab257 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "leadMaintainer": "Volker Mische ", "main": "src/index.js", "scripts": { - "lint": "aegir lint", + "lint": "aegir lint && flow check --color=always", "test": "aegir test", "test:node": "aegir test --target node", "test:browser": "aegir test --target browser", @@ -41,6 +41,7 @@ }, "devDependencies": { "aegir": "^18.2.0", + "flow-bin": "0.96.0", "chai": "^4.2.0", "dirty-chai": "^2.0.1", "multihashing": "~0.3.3", diff --git a/src/cid-util.js b/src/cid-util.js index 8adfdd7..2c938f4 100644 --- a/src/cid-util.js +++ b/src/cid-util.js @@ -1,3 +1,5 @@ +// @flow strict + 'use strict' const mh = require('multihashes') @@ -9,9 +11,9 @@ var CIDUtil = { * Returns undefined if it is a valid CID. * * @param {any} other - * @returns {string} + * @returns {?string} */ - checkCIDComponents: function (other) { + checkCIDComponents: function (other /*: any */)/*: ?string */ { if (other == null) { return 'null values are not valid CIDs' } diff --git a/src/index.js b/src/index.js index cf53eb3..300f60b 100644 --- a/src/index.js +++ b/src/index.js @@ -1,3 +1,5 @@ +// @flow strict + 'use strict' const mh = require('multihashes') @@ -7,11 +9,24 @@ const codecs = require('multicodec/src/base-table') const CIDUtil = require('./cid-util') const withIs = require('class-is') +/*:: +export type Version = 0 | 1 +export type Codec = string +export type Multihash = Buffer +export type BaseEncodedString = string +export type MultibaseName = string +export type SerializedCID = { + codec:Codec; + version:Version; + hash:Multihash; +} +*/ + /** * @typedef {Object} SerializedCID * @param {string} codec * @param {number} version - * @param {Buffer} multihash + * @param {Buffer} hash */ /** @@ -28,7 +43,7 @@ const withIs = require('class-is') * , as defined in [ipld/cid](https://github.com/multiformats/cid). * @class CID */ -class CID { +class CID /*:: */{ /** * Create a new CID. * @@ -59,10 +74,23 @@ class CID { * new CID() * new CID() */ - constructor (version, codec, multihash, multibaseName = 'base58btc') { - if (module.exports.isCID(version)) { - // version is an exising CID instance - const cid = version + /*:: + +version:Version + +codec:Codec + +multihash:Multihash + +multibaseName:MultibaseName + string:?string + _buffer:?Buffer + static isCID:(mixed) => boolean + */ + constructor ( + version/*: CID | BaseEncodedString | Buffer | Version */, + codec/*: ?Codec */ = undefined, + multihash/*: ?Multihash */ = undefined, + multibaseName/*: MultibaseName */ = 'base58btc' + ) { + const cid = CID.matchCID(version) + if (cid) { this.version = cid.version this.codec = cid.codec this.multihash = Buffer.from(cid.multihash) @@ -76,7 +104,11 @@ class CID { if (baseName) { // version is a CID String encoded with multibase, so v1 const cid = multibase.decode(version) - this.version = parseInt(cid.slice(0, 1).toString('hex'), 16) + // Type checker will fail because parseInt isn't guaranteed to return + // 0 or 1 it can be any int. Invariant is later enforced through + // `validateCID` and we use any type to make type-checker trust us. + const v/*: any */ = parseInt(cid.slice(0, 1).toString('hex'), 16) + this.version = v this.codec = multicodec.getCodec(cid.slice(1)) this.multihash = multicodec.rmPrefix(cid.slice(1)) this.multibaseName = baseName @@ -92,7 +124,10 @@ class CID { return } - if (Buffer.isBuffer(version)) { + // type checker can refine type from predicate function like `isBuffer` but + // it can on instanceof check, which is why we use inline comment to enable + // that refinement. + if (Buffer.isBuffer(version) /*:: && version instanceof Buffer */) { const firstByte = version.slice(0, 1) const v = parseInt(firstByte.toString('hex'), 16) if (v === 0 || v === 1) { @@ -114,21 +149,26 @@ class CID { } // otherwise, assemble the CID from the parameters + // type checker will not accept `version`, `codec`, `multihash` as is + // because types don't correspond to [number, string, Buffer] so we + // use any below as we know `CID.validateCID` will throw if types do not + // match up. + const [$version, $codec, $multihash]/*: any */ = [version, codec, multihash] /** * @type {number} */ - this.version = version + this.version = $version /** * @type {string} */ - this.codec = codec + this.codec = $codec /** * @type {Buffer} */ - this.multihash = multihash + this.multihash = $multihash /** * @type {string} @@ -146,7 +186,7 @@ class CID { * * @memberOf CID */ - get buffer () { + get buffer ()/*: Buffer */ { let buffer = this._buffer if (!buffer) { @@ -175,7 +215,7 @@ class CID { * @returns {Buffer} * @readonly */ - get prefix () { + get prefix ()/*: Buffer */ { return Buffer.concat([ Buffer.from(`0${this.version}`, 'hex'), multicodec.getCodeVarint(this.codec), @@ -188,7 +228,7 @@ class CID { * * @returns {CID} */ - toV0 () { + toV0 ()/*: CID */ { if (this.codec !== 'dag-pb') { throw new Error('Cannot convert a non dag-pb CID to CIDv0') } @@ -211,7 +251,7 @@ class CID { * * @returns {CID} */ - toV1 () { + toV1 ()/*: CID */ { return new _CID(1, this.codec, this.multihash) } @@ -221,7 +261,7 @@ class CID { * @param {string} [base=this.multibaseName] - Base encoding to use. * @returns {string} */ - toBaseEncodedString (base = this.multibaseName) { + toBaseEncodedString (base/*: MultibaseName */ = this.multibaseName)/*: BaseEncodedString */ { if (this.string && base === this.multibaseName) { return this.string } @@ -243,7 +283,7 @@ class CID { return str } - toString (base) { + toString (base)/*: BaseEncodedString */ { return this.toBaseEncodedString(base) } @@ -252,7 +292,7 @@ class CID { * * @returns {SerializedCID} */ - toJSON () { + toJSON ()/*: SerializedCID */ { return { codec: this.codec, version: this.version, @@ -263,13 +303,18 @@ class CID { /** * Compare equality with another CID. * - * @param {CID} other + * @param {any} other * @returns {bool} */ - equals (other) { - return this.codec === other.codec && + equals (other/*: mixed */)/*: boolean */ { + return ( + other != null && + typeof other === 'object' && + this.codec === other.codec && this.version === other.version && + other.multihash instanceof Buffer && this.multihash.equals(other.multihash) + ) } /** @@ -279,12 +324,30 @@ class CID { * @param {any} other * @returns {void} */ - static validateCID (other) { + static validateCID (other/*: mixed */)/*: void */ { let errorMsg = CIDUtil.checkCIDComponents(other) if (errorMsg) { throw new Error(errorMsg) } } + + /** + * Test if the given value is a valid CID object, if it is returns it back, + * otherwise returns undefined. + * @param {any} value + * @returns {?CID} + */ + static matchCID (value/*: mixed */)/*: ?CID */ { + if (module.exports.isCID(value)) { + // Type checker is unable to refine type through predicate function, + // but we know value is CID so we use any making type-checker trust + // our judgment. + const cid/*: any */ = value + return cid + } else { + return undefined + } + } } const _CID = withIs(CID, { diff --git a/src/index.js.flow b/src/index.js.flow deleted file mode 100644 index f4de68b..0000000 --- a/src/index.js.flow +++ /dev/null @@ -1,33 +0,0 @@ -// @flow strict - -export type Version = 0 | 1 -export type Codec = string -export type Multihash = Buffer -export type BaseEncodedString = string -export type MultibaseName = string - -declare class CID { - constructor(Version, Codec, Multihash, multibaseName?:MultibaseName): void; - constructor(BaseEncodedString): void; - constructor(Buffer): void; - - +codec: Codec; - +multihash: Multihash; - +buffer: Buffer; - +prefix: Buffer; - - toV0(): CID; - toV1(): CID; - toBaseEncodedString(base?: string): BaseEncodedString; - toString(): BaseEncodedString; - toJSON(): { codec: Codec, version: Version, hash: Multihash }; - - equals(mixed): boolean; - - static codecs: { [string]: Codec }; - static isCID(mixed): boolean; - static validateCID(mixed): void; -} - -export default CID -export type { CID } diff --git a/test/index.spec.js b/test/index.spec.js index 5544493..9b7b8af 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -212,6 +212,8 @@ describe('CID', () => { it('.equals v0 to v0', () => { expect(new CID(h1).equals(new CID(h1))).to.equal(true) expect(new CID(h1).equals(new CID(h2))).to.equal(false) + expect(new CID(h1).equals(h1)).to.equal(false) + expect(new CID(h1).equals({ string: h1 })).to.equal(false) }) it('.equals v0 to v1 and vice versa', () => { @@ -245,6 +247,36 @@ describe('CID', () => { CID.isCID(new CID(h1).toV1()) ).to.equal(true) }) + + it('.matchCID', () => { + const c1 = new CID(h1) + + expect( + CID.matchCID(c1) + ).to.equal(c1) + + expect( + CID.matchCID(h1) + ).to.equal(undefined) + + expect( + CID.matchCID(false) + ).to.equal(undefined) + + expect( + CID.matchCID(Buffer.from('hello world')) + ).to.equal(undefined) + + const c1v0 = new CID(h1).toV0() + expect( + CID.matchCID(c1v0) + ).to.equal(c1v0) + + const c1v1 = new CID(h1).toV1() + expect( + CID.matchCID(c1v1) + ).to.equal(c1v1) + }) }) describe('throws on invalid inputs', () => { From d5a7990d6c11b98c6dfeef4cd5d96304c3307ba8 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Tue, 16 Apr 2019 14:07:47 -0700 Subject: [PATCH 2/5] chore: remove .prettierignore to address review comment --- .prettierignore | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 .prettierignore diff --git a/.prettierignore b/.prettierignore deleted file mode 100644 index 7988935..0000000 --- a/.prettierignore +++ /dev/null @@ -1,2 +0,0 @@ -**/*.js -!**/*.flow.js \ No newline at end of file From 19cd9458a42c7379d7b836116331c1cf6aab903c Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Tue, 16 Apr 2019 14:09:50 -0700 Subject: [PATCH 3/5] fix: move type checknig into own script (as per review) --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 0fab257..2bddb57 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,8 @@ "leadMaintainer": "Volker Mische ", "main": "src/index.js", "scripts": { - "lint": "aegir lint && flow check --color=always", + "lint": "aegir lint", + "check": "flow check --color=always", "test": "aegir test", "test:node": "aegir test --target node", "test:browser": "aegir test --target browser", From 41f3f111ac726dc70a552b4e6fd7cb82b10133b0 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Tue, 16 Apr 2019 14:12:36 -0700 Subject: [PATCH 4/5] docs: explain CID type parameter --- src/index.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/index.js b/src/index.js index 300f60b..4cc11b4 100644 --- a/src/index.js +++ b/src/index.js @@ -43,6 +43,18 @@ export type SerializedCID = { * , as defined in [ipld/cid](https://github.com/multiformats/cid). * @class CID */ +// CID is generic and type parameter `a` represents type of the data that +// the CID addresses. While type parameter is not really used in this library +// it is still useful in enabling other libraries to encode type information +// e.g. it is possible to define functions like: +// function get (CID):Promise +// function put (a):Promise> +// Which would allow type-checker to infer type of the following value: +// const cid = await put({ x: 1, y: 2 }) +// ... +// const point = await get(cid) +// point.x //:number +// point.z //:Cannot get `point.z` because property `z` is missing in object literal { x: 1, y: 2 }. class CID /*:: */{ /** * Create a new CID. From 1bc16700542c4289236bb8508154b2d0abb23d55 Mon Sep 17 00:00:00 2001 From: Irakli Gozalishvili Date: Wed, 17 Apr 2019 11:36:06 -0700 Subject: [PATCH 5/5] fix: run type check on push & pull --- .travis.yml | 1 + package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index d0cf281..59af74c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,6 +23,7 @@ jobs: - npx aegir commitlint --travis - npx aegir dep-check - npm run lint + - npm run check - stage: test name: chrome diff --git a/package.json b/package.json index 2bddb57..06cf100 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ }, "pre-push": [ "lint", + "check", "test" ], "repository": {