Skip to content

Commit

Permalink
fix: throw when unsupported fields are detected (#80)
Browse files Browse the repository at this point in the history
`required` is a field in proto2 but not in proto3 so throw if it
is encountered while generating `.ts` from `.proto`.

It would be better to detect `proto2` from the `syntax` directive
but it's not in the output of the `pbjs -t json` command.

Fixes #34
  • Loading branch information
achingbrain committed Jan 31, 2023
1 parent 3ac2c56 commit 8108875
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 8 deletions.
2 changes: 1 addition & 1 deletion packages/protons-benchmark/src/decode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ new Benchmark.Suite()
})
.on('complete', function () {
// @ts-expect-error types are wrong
console.info(`Fastest is ${this.filter('fastest').map('name')}`) // eslint-disable-line @typescript-eslint/restrict-template-expressions
console.info(`Fastest is ${this.filter('fastest').map('name')}`)
})
// run async
.run({ async: true })
2 changes: 1 addition & 1 deletion packages/protons-benchmark/src/encode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ new Benchmark.Suite()
})
.on('complete', function () {
// @ts-expect-error types are wrong
console.info(`Fastest is ${this.filter('fastest').map('name')}`) // eslint-disable-line @typescript-eslint/restrict-template-expressions
console.info(`Fastest is ${this.filter('fastest').map('name')}`)
})
// run async
.run({ async: true })
2 changes: 1 addition & 1 deletion packages/protons-benchmark/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ new Benchmark.Suite()
})
.on('complete', function () {
// @ts-expect-error types are wrong
console.info(`Fastest is ${this.filter('fastest').map('name')}`) // eslint-disable-line @typescript-eslint/restrict-template-expressions
console.info(`Fastest is ${this.filter('fastest').map('name')}`)
})
// run async
.run({ async: true })
2 changes: 1 addition & 1 deletion packages/protons-benchmark/src/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ new Benchmark.Suite()
})
.on('complete', function () {
// @ts-expect-error types are wrong
console.info(`Fastest is ${this.filter('fastest').map('name')}`) // eslint-disable-line @typescript-eslint/restrict-template-expressions
console.info(`Fastest is ${this.filter('fastest').map('name')}`)
})
// run async
.run({ async: true })
3 changes: 1 addition & 2 deletions packages/protons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@
"pbjs": "^0.0.14",
"protobufjs": "^7.0.0",
"protons-runtime": "^4.0.0",
"uint8arraylist": "^2.3.2",
"uint8arrays": "^4.0.2"
"uint8arraylist": "^2.3.2"
}
}
4 changes: 4 additions & 0 deletions packages/protons/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,10 @@ function defineModule (def: ClassDef): ModuleDef {
fieldDef.repeated = fieldDef.rule === 'repeated'
fieldDef.optional = !fieldDef.repeated && fieldDef.options?.proto3_optional === true
fieldDef.map = fieldDef.keyType != null

if (fieldDef.rule === 'required') {
throw new Error('"required" fields are not allowed in proto3 - please convert your proto2 definitions to proto3')
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions packages/protons/test/fixtures/proto2.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
syntax = "proto2";

message Message {
required string requiredField = 1;
}
1 change: 0 additions & 1 deletion packages/protons/test/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-env mocha */
/* eslint-disable @typescript-eslint/restrict-template-expressions */

import { expect } from 'aegir/chai'
import pbjs from 'pbjs'
Expand Down
1 change: 0 additions & 1 deletion packages/protons/test/maps.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-env mocha */
/* eslint-disable @typescript-eslint/restrict-template-expressions */

import { expect } from 'aegir/chai'
import { MapTypes, SubMessage } from './fixtures/maps.js'
Expand Down
11 changes: 11 additions & 0 deletions packages/protons/test/unsupported.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* eslint-env mocha */

import { expect } from 'aegir/chai'
import { generate } from '../src/index.js'

describe('unsupported', () => {
it('should refuse to generate source from proto2 definition', async () => {
await expect(generate('test/fixtures/proto2.proto', {})).to.eventually.be.rejected
.with.property('message').that.contain('"required" fields are not allowed in proto3')
})
})

0 comments on commit 8108875

Please sign in to comment.