PeculiarVentures / PKI.js

PKI.js is a pure JavaScript library implementing the formats that are used in PKI applications (signing, encryption, certificate requests, OCSP and TSP requests/responses). It is built on WebCrypto (Web Cryptography API) and requires no plug-ins.
http://pkijs.org
Other
1.25k stars 204 forks source link

Remove unused encodeFlag argument #280

Closed lpinca closed 4 years ago

lpinca commented 4 years ago

The toSchema() method of these classes takes no arguments.

YuryStrozhevsky commented 4 years ago

@lpinca Why do you think the "encodeFlag" is "unused"?

YuryStrozhevsky commented 4 years ago

@lpinca At least CRL class is using the flag. But anyway I do not see a reason to remove it from code - probably in a future it would be necessary.

lpinca commented 4 years ago

https://github.com/PeculiarVentures/PKI.js/blob/0c3f10839098e6cebfde0f06ab4c7835d265ad16/src/AlgorithmIdentifier.js#L145 takes no arguments

https://github.com/PeculiarVentures/PKI.js/blob/0c3f10839098e6cebfde0f06ab4c7835d265ad16/src/OtherRevocationInfoFormat.js#L118

takes no arguments

https://github.com/PeculiarVentures/PKI.js/blob/0c3f10839098e6cebfde0f06ab4c7835d265ad16/src/SignerInfo.js#L276

takes no arguments


This is confusing to anyone trying to understand why the encodeFlag argument of SignedData.prototype.toSchema() is passed to AlgorithmIdentifier.prototype.toSchema() for example.

In my opinion it should be added only when AlgorithmIdentifier.prototype.toSchema() and the others support it.

lpinca commented 4 years ago

Just to make my point clearer. This does not work in TypeScript and many other languages of course.

function bar() {
  return 10;
}

function foo(n: number) {
  return bar(n) + n;
}

console.log(foo(5));

The compiler returns the following error:

tsc index.ts
index.ts:6:14 - error TS2554: Expected 0 arguments, but got 1.

6   return bar(n) + n;
               ~

Found 1 error.