Resolve JS-81- Options Schemas required for ESLint v9 #4679
Conversation
ericmorand-sonarsource
force-pushed
the
JS-81
branch
12 times, most recently
from
1ede563 to
49ff037
Compare
ericmorand-sonarsource
changed the title
ericmorand-sonarsource
force-pushed
the
JS-81
branch
5 times, most recently
from
10a4d8a to
3f86fda
Compare
ericmorand-sonarsource
force-pushed
the
JS-81
branch
from
3f86fda to
5843c5f
Compare
There was a problem hiding this comment.
Previously, there was no test at all for this one.
There was a problem hiding this comment.
Talked offline about the typing - very slick.
The Java class serve no purpose, except for storing the default values. Except for that, they could be autogenerated. If we were to move these definitions into js/ts, we could fully auto generate them and remove the need for the additional tests to test passing of simple values....
All in all looks good and a very positive change!
| string?, | ||
| ]; | ||
|
|
||
| export const rule: RuleModule<Options> = { |
There was a problem hiding this comment.
As discussed offline, perhaps we could introduce a new type, when the rule depends on the "SONAR_RUNTIME" check and then have the enum be added on implicitly?
| @@ -25,21 +25,36 @@ import { TSESTree } from '@typescript-eslint/utils'; | |||
| import { getMainFunctionTokenLocation } from 'eslint-plugin-sonarjs/lib/utils/locations'; | |||
| import { SONAR_RUNTIME } from '../../linter/parameters'; | |||
| import { RuleContext, toEncodedMessage } from '../helpers'; | |||
| import type { RuleModule } from '../../../../shared/src/types/rule'; | |||
|
|
|||
| const DEFAULT_THRESHOLD = 4; | |||
There was a problem hiding this comment.
I do hope so, that the Java default is the same as this default... But in all seriousness perhaps this default could be further integrated into the js/ts rule? Perhaps in the meta field somewhere? As we discussed with @ericmorand-sonarsource, the default parameters are currently stored on the Java side, which unnecessarily scatters the logic into the boilerplate Java file... Perhaps a Jira task for the future?
There was a problem hiding this comment.
I definitely agree.
sonar-plugin/javascript-checks/src/main/java/org/sonar/javascript/checks/GetterSetterCheck.java
Show resolved
Hide resolved
...n/javascript-checks/src/main/java/org/sonar/javascript/checks/HardcodedCredentialsCheck.java
Show resolved
Hide resolved
ericmorand-sonarsource
force-pushed
the
JS-81
branch
from
3504e22 to
546faa7
Compare
ericmorand-sonarsource
force-pushed
the
JS-81
branch
from
546faa7 to
94f1094
Compare
|
| type RuleModuleSchema<Options extends RuleOptions> = | ||
| | { | ||
| type: 'object'; | ||
| properties: { [key in keyof Options[0]]: JSONSchema.JSONSchema4 }; |
There was a problem hiding this comment.
@ericmorand-sonarsource - I was looking through this and have a q: What happens if the RuleOptions is only [SonarRuntime], what would be this allowed this "key in keyof Options[0]" ?
There was a problem hiding this comment.
Probably not, we would have to set schema to:
schema: [
{
type: 'object',
properties: {},
},
{
type: 'string',
enum: ['sonar-runtime']
}
]Which is what we have to put as a minimal schema, anyway, since we can't know if a rule will eventually have options.
Just testing from the CI to prevent my computer from crashing.
Fixes JS-81