diff --git a/.eslintrc.js b/.eslintrc.js index 755e50bf20b1..f5f3e2f53b9f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -176,5 +176,11 @@ module.exports = { "@typescript-eslint/*": "off", }, }, + { + files: ["packages/cc/src/**/*CC.ts"], + rules: { + "@zwave-js/ccapi-validate-args": "error", + }, + }, ], }; diff --git a/.github/workflows/test-and-release.yml b/.github/workflows/test-and-release.yml index 36984931bf77..4d7683154067 100644 --- a/.github/workflows/test-and-release.yml +++ b/.github/workflows/test-and-release.yml @@ -138,6 +138,11 @@ jobs: node-version: ${{ matrix.node-version }} githubToken: ${{ secrets.GITHUB_TOKEN }} + # For linting to succeed, we need to build the eslint plugin first. + # Thanks to caching from the previous job, this should be almost a no-op + - name: Compile TypeScript code + run: yarn build $TURBO_FLAGS + - name: Run linters run: yarn run lint $TURBO_FLAGS diff --git a/.vscode/settings.json b/.vscode/settings.json index 0734420e9be2..f8d32aa35b25 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -6,8 +6,12 @@ "source.fixAll.eslint", "source.formatDocument" ], + "eslint.codeActionsOnSave.mode": "problems", "eslint.codeActionsOnSave.rules": [ - "!@zwave-js/no-debug-in-tests" // Don't auto-fix this rule on save, or we cannot use debug mode in tests + "!@zwave-js/no-debug-in-tests", // Don't auto-fix this rule on save, or we cannot use debug mode in tests + "!@zwave-js/ccapi-validate-args", // We may want to decide whether to fix this or not on a case-by-case basis + "!report-unused-eslint-disable-comments", // Without this, disables for the above rules would be removed + "*" ], "eslint.rules.customizations": [ { diff --git a/maintenance/build.ts b/maintenance/build.ts index 6a2dba5fd52f..402960354346 100644 --- a/maintenance/build.ts +++ b/maintenance/build.ts @@ -11,9 +11,10 @@ const buildArgs = process.argv // Only cc, config and projects that depend on them need codegen and partial builds const needsNoCodegen = [ "@zwave-js/maintenance", + "@zwave-js/nvmedit", "@zwave-js/core", + "@zwave-js/eslint-plugin", "@zwave-js/shared", - "@zwave-js/nvmedit", "@zwave-js/transformers", ]; @@ -22,6 +23,8 @@ const hasCodegen = ["@zwave-js/cc", "@zwave-js/config"]; // zwave-js is the main entry point, but there are projects that depend on it const dependsOnZwaveJs = [ "@zwave-js/flash", + // The eslint plugin doesn't actually depend on zwave-js, but it needs to be built too + "@zwave-js/eslint-plugin", // And CLI in the future ]; diff --git a/packages/cc/maintenance/_tasks.ts b/packages/cc/maintenance/_tasks.ts index 7f2a7f8dfe8c..ab0b66e101d6 100644 --- a/packages/cc/maintenance/_tasks.ts +++ b/packages/cc/maintenance/_tasks.ts @@ -4,7 +4,6 @@ import { generateCCExports } from "./generateCCExports"; import { generateCCValuesInterface } from "./generateCCValuesInterface"; // import { lintCCConstructors } from "./lintCCConstructor"; import { lintCCInterview } from "./lintCCInterview"; -import { lintCCValidateArgs } from "./lintCCValidateArgs"; const argv = process.argv.slice(2); @@ -12,7 +11,6 @@ const lint = () => Promise.all([ lintCCInterview(), // lintCCConstructors(), - lintCCValidateArgs(), ]); const codegen = () => Promise.all([ diff --git a/packages/cc/maintenance/lintCCValidateArgs.ts b/packages/cc/maintenance/lintCCValidateArgs.ts deleted file mode 100644 index 3045efb6720b..000000000000 --- a/packages/cc/maintenance/lintCCValidateArgs.ts +++ /dev/null @@ -1,183 +0,0 @@ -/*! - * This scripts checks that all CC API classes have a @noValidateArgs decorator on their methods which need one - */ - -import { getCCName } from "@zwave-js/core"; -import { - getCommandClassFromDecorator, - hasComment, - loadTSConfig, - projectRoot, - reportProblem, -} from "@zwave-js/maintenance"; -import { blue, green } from "ansi-colors"; -import * as path from "path"; -import ts from "typescript"; - -function hasNoValidateArgsComment( - node: ts.Node, - sourceFile: ts.SourceFile, -): boolean { - return hasComment( - sourceFile, - node, - (text) => text.includes("@noValidateArgs"), - ); -} - -function hasInternalJsDoc(node: ts.Node, sourceFile: ts.SourceFile): boolean { - return hasComment( - sourceFile, - node, - (text, kind) => - text.includes("@internal") - && kind === ts.SyntaxKind.MultiLineCommentTrivia, - ); -} - -export function lintCCValidateArgs(): Promise { - // Create a Program to represent the project, then pull out the - // source file to parse its AST. - - const tsConfig = loadTSConfig("cc"); - const program = ts.createProgram(tsConfig.fileNames, tsConfig.options); - - let hasError = false; - - // Scan all source files - for (const sourceFile of program.getSourceFiles()) { - const relativePath = path - .relative(projectRoot, sourceFile.fileName) - .replace(/\\/g, "/"); - - // Only look at files in this package - if (relativePath.startsWith("..")) continue; - - // Only look at *CC.ts files in the lib dir - if ( - !relativePath.includes("/src/lib/") - || !relativePath.endsWith("CC.ts") - ) { - continue; - } - - ts.forEachChild(sourceFile, (node) => { - // Only look at class decorations that are annotated with @API and don't have a // @noValidateArgs comment - if (!ts.isClassDeclaration(node)) return; - if (!node.decorators) return; - if (hasNoValidateArgsComment(node, sourceFile)) return; - - const cc = node.decorators - .filter( - (d) => - ts.isCallExpression(d.expression) - && ts.isIdentifier(d.expression.expression) - && d.expression.expression.text === "API", - ) - .map((d) => getCommandClassFromDecorator(sourceFile, d)) - .find((cc) => cc != undefined); - if (!cc) return; - - // Check all public method declarations with arguments that are not called supportsCommand - const methods = node.members - .filter( - (m): m is ts.MethodDeclaration => - ts.isMethodDeclaration(m) - // Ignore overload declarations - && !!m.body - && m.parameters.length > 0, - ) - .filter( - (m) => - ts.isIdentifier(m.name) - && m.name.text !== "supportsCommand" - && m.name.text !== "isSetValueOptimistic", - ) - .filter((m) => - m.modifiers?.some( - (mod) => mod.kind === ts.SyntaxKind.PublicKeyword, - ) - ) - // Ignore methods marked with @internal - .filter((m) => !hasInternalJsDoc(m, sourceFile)); - - if (methods.length === 0) { - // ignore empty classes - return; - } else { - for (const method of methods) { - const methodLocation = ts.getLineAndCharacterOfPosition( - sourceFile, - method.getStart(sourceFile, false), - ); - const fail = ( - reason: string, - severity: "error" | "warn" = "error", - ) => { - if (severity === "error") hasError = true; - reportProblem({ - severity, - filename: relativePath, - line: methodLocation.line + 1, - message: reason, - }); - }; - - if (hasNoValidateArgsComment(method, sourceFile)) { - // ignored - return; - } else { - const hasValidateArgsDecorator = !!method.decorators - ?.some( - (d) => - ts.isCallExpression(d.expression) - && ts.isIdentifier(d.expression.expression) - && d.expression.expression.text - === "validateArgs", - ); - if (!hasValidateArgsDecorator) { - fail( - `The API class for the ${ - blue( - getCCName(cc), - ) - } CC is missing the ${ - blue( - "@validateArgs()", - ) - } decorator on the ${ - blue( - (method.name as ts.Identifier).text, - ) - } method. -Public CC API methods should have argument validation to catch user errors. -If this is a false-positive, consider suppressing this error with a ${ - green( - "// @noValidateArgs", - ) - } comment before the method implementation.`, - "error", - ); - } - } - } - } - }); - } - - if (hasError) { - return Promise.reject( - new Error( - "Linting the CC API method validations was not successful! See log output for details.", - ), - ); - } else { - return Promise.resolve(); - } -} - -if (require.main === module) { - lintCCValidateArgs() - .then(() => process.exit(0)) - .catch(() => process.exit(1)); -} diff --git a/packages/cc/src/cc/AssociationCC.ts b/packages/cc/src/cc/AssociationCC.ts index f7c8dd6d198e..913154c08ee6 100644 --- a/packages/cc/src/cc/AssociationCC.ts +++ b/packages/cc/src/cc/AssociationCC.ts @@ -113,6 +113,7 @@ export class AssociationCCAPI extends PhysicalCCAPI { if (response) return response.groupCount; } + @validateArgs() public async reportGroupCount(groupCount: number): Promise { this.assertSupportsCommand( AssociationCommand, @@ -152,6 +153,7 @@ export class AssociationCCAPI extends PhysicalCCAPI { } } + @validateArgs() public async sendReport( options: AssociationCCReportSpecificOptions, ): Promise { diff --git a/packages/cc/src/cc/AssociationGroupInfoCC.ts b/packages/cc/src/cc/AssociationGroupInfoCC.ts index ddde285476ab..f542f80438a3 100644 --- a/packages/cc/src/cc/AssociationGroupInfoCC.ts +++ b/packages/cc/src/cc/AssociationGroupInfoCC.ts @@ -168,6 +168,7 @@ export class AssociationGroupInfoCCAPI extends PhysicalCCAPI { } } + @validateArgs() public async reportGroupInfo( options: AssociationGroupInfoCCInfoReportSpecificOptions, ): Promise { diff --git a/packages/cc/src/cc/CRC16CC.ts b/packages/cc/src/cc/CRC16CC.ts index 7d38da7905b5..545da3fec61d 100644 --- a/packages/cc/src/cc/CRC16CC.ts +++ b/packages/cc/src/cc/CRC16CC.ts @@ -26,8 +26,10 @@ import { CRC16Command } from "../lib/_Types"; // @noSetValueAPI // @noInterview This CC only has a single encapsulation command -// @noValidateArgs - Encapsulation CCs are used internally and too frequently that we +// Encapsulation CCs are used internally and too frequently that we // want to pay the cost of validating each call +/* eslint-disable @zwave-js/ccapi-validate-args */ + @API(CommandClasses["CRC-16 Encapsulation"]) export class CRC16CCAPI extends CCAPI { public supportsCommand(_cmd: CRC16Command): MaybeNotKnown { diff --git a/packages/cc/src/cc/InclusionControllerCC.ts b/packages/cc/src/cc/InclusionControllerCC.ts index f1ad800fe4c0..d322f44923c5 100644 --- a/packages/cc/src/cc/InclusionControllerCC.ts +++ b/packages/cc/src/cc/InclusionControllerCC.ts @@ -25,6 +25,9 @@ import { InclusionControllerStep, } from "../lib/_Types"; +// This CC should not be used directly from user code +/* eslint-disable @zwave-js/ccapi-validate-args */ + @commandClass(CommandClasses["Inclusion Controller"]) @implementedVersion(1) export class InclusionControllerCC extends CommandClass { diff --git a/packages/cc/src/cc/IndicatorCC.ts b/packages/cc/src/cc/IndicatorCC.ts index fe27e691c9cb..8013a6a41b07 100644 --- a/packages/cc/src/cc/IndicatorCC.ts +++ b/packages/cc/src/cc/IndicatorCC.ts @@ -477,6 +477,7 @@ export class IndicatorCCAPI extends CCAPI { * - an object specifying the timeout parts. An empty object will be treated like `undefined`. * - `undefined` to disable the timeout. */ + @validateArgs() public async setTimeout( indicatorId: number, timeout: IndicatorTimeout | string | undefined, @@ -591,6 +592,7 @@ export class IndicatorCCAPI extends CCAPI { /** * Returns the timeout after which the given indicator will be turned off. */ + @validateArgs() public async getTimeout( indicatorId: number, ): Promise> { diff --git a/packages/cc/src/cc/MultiChannelCC.ts b/packages/cc/src/cc/MultiChannelCC.ts index d5706ec23030..c83c75174e65 100644 --- a/packages/cc/src/cc/MultiChannelCC.ts +++ b/packages/cc/src/cc/MultiChannelCC.ts @@ -329,8 +329,9 @@ export class MultiChannelCCAPI extends CCAPI { return response?.members; } - // @noValidateArgs - Encapsulation is used internally and too frequently that we + // Encapsulation is used internally and too frequently that we // want to pay the cost of validating each call + // eslint-disable-next-line @zwave-js/ccapi-validate-args public async sendEncapsulated( options: Omit< MultiChannelCCCommandEncapsulationOptions, @@ -371,8 +372,9 @@ export class MultiChannelCCAPI extends CCAPI { return response?.endpointCount; } - // @noValidateArgs - Encapsulation is used internally and too frequently that we + // Encapsulation is used internally and too frequently that we // want to pay the cost of validating each call + // eslint-disable-next-line @zwave-js/ccapi-validate-args public async sendEncapsulatedV1(encapsulated: CommandClass): Promise { this.assertSupportsCommand( MultiChannelCommand, diff --git a/packages/cc/src/cc/NotificationCC.ts b/packages/cc/src/cc/NotificationCC.ts index fe291c0205d3..9b0077ea364f 100644 --- a/packages/cc/src/cc/NotificationCC.ts +++ b/packages/cc/src/cc/NotificationCC.ts @@ -387,7 +387,7 @@ export class NotificationCCAPI extends PhysicalCCAPI { } } -function getNotificationEnumBehavior( +export function getNotificationEnumBehavior( notificationConfig: Notification, valueConfig: NotificationValueDefinition & { type: "state" }, ): "none" | "extend" | "replace" { @@ -444,9 +444,10 @@ export function getNotificationValueMetadata( } if (valueConfig.parameter instanceof NotificationParameterWithEnum) { for (const [value, label] of valueConfig.parameter.values) { - metadata.states![ - getNotificationStateValueWithEnum(valueConfig.value, value) - ] = label; + const stateKey = enumBehavior === "replace" + ? value + : getNotificationStateValueWithEnum(valueConfig.value, value); + metadata.states![stateKey] = label; } } diff --git a/packages/cc/src/cc/Security2CC.ts b/packages/cc/src/cc/Security2CC.ts index b531cbf1c1b3..6042efa698c1 100644 --- a/packages/cc/src/cc/Security2CC.ts +++ b/packages/cc/src/cc/Security2CC.ts @@ -149,8 +149,10 @@ interface DecryptionResult { securityClass: SecurityClass | undefined; } -// @noValidateArgs - Encapsulation CCs are used internally and too frequently that we +// Encapsulation CCs are used internally and too frequently that we // want to pay the cost of validating each call +/* eslint-disable @zwave-js/ccapi-validate-args */ + @API(CommandClasses["Security 2"]) export class Security2CCAPI extends CCAPI { public supportsCommand(_cmd: Security2Command): MaybeNotKnown { diff --git a/packages/cc/src/cc/SecurityCC.ts b/packages/cc/src/cc/SecurityCC.ts index aed41b976b81..cb731aefb493 100644 --- a/packages/cc/src/cc/SecurityCC.ts +++ b/packages/cc/src/cc/SecurityCC.ts @@ -77,8 +77,10 @@ const HALF_NONCE_SIZE = 8; // TODO: Ignore commands if received via multicast -// @noValidateArgs - Encapsulation CCs are used internally and too frequently that we +// Encapsulation CCs are used internally and too frequently that we // want to pay the cost of validating each call +/* eslint-disable @zwave-js/ccapi-validate-args */ + @API(CommandClasses.Security) export class SecurityCCAPI extends PhysicalCCAPI { public supportsCommand(_cmd: SecurityCommand): MaybeNotKnown { diff --git a/packages/cc/src/cc/SupervisionCC.ts b/packages/cc/src/cc/SupervisionCC.ts index 79de773d6070..98d262ef17ef 100644 --- a/packages/cc/src/cc/SupervisionCC.ts +++ b/packages/cc/src/cc/SupervisionCC.ts @@ -55,8 +55,10 @@ export const SupervisionCCValues = Object.freeze({ // @noSetValueAPI - This CC has no values to set // @noInterview - This CC is only used for encapsulation -// @noValidateArgs - Encapsulation CCs are used internally and too frequently that we +// Encapsulation CCs are used internally and too frequently that we // want to pay the cost of validating each call +/* eslint-disable @zwave-js/ccapi-validate-args */ + @API(CommandClasses.Supervision) export class SupervisionCCAPI extends PhysicalCCAPI { public supportsCommand(cmd: SupervisionCommand): MaybeNotKnown { diff --git a/packages/cc/src/cc/TimeCC.ts b/packages/cc/src/cc/TimeCC.ts index d21610f7db6f..37d6164441f8 100644 --- a/packages/cc/src/cc/TimeCC.ts +++ b/packages/cc/src/cc/TimeCC.ts @@ -71,6 +71,7 @@ export class TimeCCAPI extends CCAPI { } } + @validateArgs() public async reportTime( hour: number, minute: number, @@ -105,6 +106,7 @@ export class TimeCCAPI extends CCAPI { } } + @validateArgs() public async reportDate( year: number, month: number, diff --git a/packages/config/config/devices/0x0460/qnsw-001x16.json b/packages/config/config/devices/0x0460/qnsw-001x16.json index 68b1167ded57..06935e685317 100644 --- a/packages/config/config/devices/0x0460/qnsw-001x16.json +++ b/packages/config/config/devices/0x0460/qnsw-001x16.json @@ -1,13 +1,12 @@ -// TODO: This file contains a placeholder for a productType, productID, or manufacturerId (0x9999) that must be corrected. { + "manufacturer": "Shelly", "manufacturerId": "0x0460", - "label": "QNSW-001X16", - "description": "Wave 1", + "label": "QNSW-001P16", + "description": "Wave 1PM", "devices": [ { - "productType": "0x9999", - "productId": "0x0002", - "zwaveAllianceId": 4922 + "productType": "0x0002", + "productId": "0x0084", } ], "firmwareVersion": { @@ -29,31 +28,58 @@ "maxNodes": 9 } }, + "paramInformation": [ { "#": "1", "label": "Sw1 Switch Type", - "description": "0", + "description": "This parameter defines how the Device should treat the switch (which type) connected to the SW (SW1) terminal.", "valueSize": 1, "minValue": 0, "maxValue": 2, "defaultValue": 2, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "mono-stable switch type (push button)", + "value": 0 + }, + { + "label": "toggle switch (contact closed - ON / contact opened - OFF)", + "value": 1 + }, + { + "label": "toggle switch (Device changes status when switch changes status)", + "value": 2 + } + ] }, { "#": "17", "label": "Restore State of O1 After Power Failure", - "description": "0", + "description": "This parameter determines if the on/off status is saved and restored for the load connected to O (O1) after a power failure.", "valueSize": 1, "minValue": 0, "maxValue": 1, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "Device saves last on/off status and restores it after a power failure", + "value": 0 + }, + { + "label": "Device does not save on/off status and does not restore it after a power failure, it remains off", + "value": 1 + } + ] }, { "#": "19", "label": "Turn O1 Off Automatically With Timer", - "description": "0", + "description": "If the load O (O1) is ON, you can schedule it to turn OFF automatically after the period of time defined in this parameter. The timer resets to zero each time the Device receives an ON command", "valueSize": 2, "minValue": 0, "maxValue": 32535, @@ -63,7 +89,7 @@ { "#": "20", "label": "Turn O1 On Automatically With Timer", - "description": "0", + "description": "If the load O (O1) is OFF, you can schedule it to turn ON automatically after the period of time defined in this parameter. The timer resets to zero each time the Device receives an OFF command", "valueSize": 2, "minValue": 0, "maxValue": 32535, @@ -73,102 +99,217 @@ { "#": "23", "label": "O1 No/Nc", - "description": "0", + "description": "The set value determines the relay contact type for output O (O1). The relay contact type can be normally open (NO) or normally closed (NC).", "valueSize": 1, "minValue": 0, "maxValue": 1, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "NO", + "value": 0 + }, + { + "label": "NC", + "value": 1 + } + ] }, { "#": "25", "label": "Set Timer Units to Seconds Or Milliseconds For O1", - "description": "0", + "description": "Set the timer units to seconds or milliseconds. Choose if you want to set the timer in seconds or milliseconds in Parameters No. 19, 20.", "valueSize": 1, "minValue": 0, "maxValue": 1, "defaultValue": 0, + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "timer set in seconds", + "value": 0 + }, + { + "label": "timer set in milliseconds (scale 10ms)", + "value": 1 + } + ] + }, + { + "#": "36", + "label": "O (O1) Power report on change - percentage", + "description": "This parameter determines the minimum change in consumed power that will result in sending a new report to the gateway.", + "unit": "%", + "valueSize": 1, + "minValue": 0, + "maxValue": 100, + "defaultValue": 50, + "unsigned": true + }, + { + "#": "39", + "label": "Minimum time between reports (O) O1", + "description": "This parameter determines the minimum time that must elapse before a new power report on O (O1) is sent to the gateway.", + "unit": "seconds", + "valueSize": 1, + "minValue": 0, + "maxValue": 120, + "defaultValue": 30, "unsigned": true }, { "#": "91", "label": "Alarm Conf. - Water", - "description": "0", + "description": "This parameter determines which alarm frames the Device should respond to and how. The parameters consist of 4 bytes, the three most significant bytes are set according to the official Z-Wave protocol specification.", "valueSize": 4, "minValue": 0, "maxValue": 2, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "no action", + "value": 0 + }, + { + "label": "open relay", + "value": 1 + }, + { + "label": "close relay", + "value": 2 + } + ] }, { "#": "92", "label": "Alarm Conf. - Smoke", - "description": "0", + "description": "This parameter determines which alarm frames the Device should respond to and how. The parameters consist of 4 bytes, the three most significant bytes are set according to the official Z-Wave protocol specification.0", "valueSize": 4, "minValue": 0, "maxValue": 2, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "no action", + "value": 0 + }, + { + "label": "open relay", + "value": 1 + }, + { + "label": "close relay", + "value": 2 + } + ] }, { "#": "93", "label": "Alarm Conf. - Co", - "description": "0", + "description": "This parameter determines which alarm frames the Device should respond to and how. The parameters consist of 4 bytes, the three most significant bytes are set according to the official Z-Wave protocol specification.", "valueSize": 4, "minValue": 0, "maxValue": 2, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "no action", + "value": 0 + }, + { + "label": "open relay", + "value": 1 + }, + { + "label": "close relay", + "value": 2 + } + ] }, { "#": "94", "label": "Alarm Conf. - Heat", - "description": "0", + "description": "This parameter determines which alarm frames the Device should respond to and how. The parameters consist of 4 bytes, the three most significant bytes are set according to the official Z-Wave protocol specification.", "valueSize": 4, "minValue": 0, "maxValue": 2, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "no action", + "value": 0 + }, + { + "label": "open relay", + "value": 1 + }, + { + "label": "close relay", + "value": 2 + } + ] }, { "#": "120", "label": "Factory Reset", - "description": "0", + "description": "Reset to factory default settings and removed from the Z-Wave network.\nThe parameter is Advanced and may be hidden under the Advanced tag.", "valueSize": 1, "minValue": 0, "maxValue": 1, "defaultValue": 0, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, + "options": [ + { + "label": "Don’t do Factory reset", + "value": 0 + }, + { + "label": "Do the Factory reset", + "value": 1 + } + ] }, { "#": "201", "label": "Serial Number 1", - "description": "0", + "description": "This parameter contains a part of device’s serial number.\nThe parameter is Read-Only and cannot be changed.\nThe parameter is Advanced and may be hidden under the Advanced tag.", "valueSize": 4, "minValue": 0, "maxValue": 2147483647, - "defaultValue": 2147483647, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, }, { "#": "202", "label": "Serial Number 2", - "description": "0", + "description": "This parameter contains a part of device’s serial number.\nThe parameter is Read-Only and cannot be changed.\nThe parameter is Advanced and may be hidden under the Advanced tag.", "valueSize": 4, "minValue": 0, "maxValue": 2147483647, - "defaultValue": 2147483647, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, }, { "#": "203", "label": "Serial Number 3", - "description": "0", + "description": "This parameter contains a part of device’s serial number\nThe parameter is Read-Only and cannot be changed.\nThe parameter is Advanced and may be hidden under the Advanced tag.", "valueSize": 4, "minValue": 0, "maxValue": 2147483647, - "defaultValue": 2147483647, - "unsigned": true + "unsigned": true, + "allowManualEntry": false, } ], "metadata": { diff --git a/packages/eslint-plugin/lib/index.js b/packages/eslint-plugin/lib/index.js deleted file mode 100644 index 64b3fa78098d..000000000000 --- a/packages/eslint-plugin/lib/index.js +++ /dev/null @@ -1,7 +0,0 @@ -const noDebugInTests = require("./no-debug-in-tests.js"); - -module.exports = { - rules: { - "no-debug-in-tests": noDebugInTests, - }, -}; diff --git a/packages/eslint-plugin/lib/utils.js b/packages/eslint-plugin/lib/utils.js deleted file mode 100644 index 27fa953370df..000000000000 --- a/packages/eslint-plugin/lib/utils.js +++ /dev/null @@ -1,9 +0,0 @@ -const path = require("node:path"); - -const repoRoot = path.normalize( - __dirname.slice(0, __dirname.lastIndexOf(`${path.sep}packages${path.sep}`)), -); - -module.exports = { - repoRoot, -}; diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index edc9f60d134f..d35cf6801851 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -4,9 +4,9 @@ "description": "zwave-js: custom ESLint rules", "private": true, "keywords": [], - "main": "lib/index.js", + "main": "build/index.js", "files": [ - "lib" + "build/**/*.js" ], "author": { "name": "AlCalzone", @@ -27,6 +27,12 @@ "engines": { "node": ">= 18" }, + "scripts": { + "build": "tsc -b tsconfig.json --pretty", + "clean": "del-cli build/ \"*.tsbuildinfo\"", + "lint:ts": "eslint --cache --ext .ts \"src/**/*.ts\"", + "lint:ts:fix": "yarn run lint:ts --fix" + }, "devDependencies": { "@typescript-eslint/utils": "^6.7.0", "typescript": "5.2.2" diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts new file mode 100644 index 000000000000..bb7ae98f3943 --- /dev/null +++ b/packages/eslint-plugin/src/index.ts @@ -0,0 +1,9 @@ +import { ccAPIValidateArgs } from "./rules/ccapi-validate-args.js"; +import { noDebugInTests } from "./rules/no-debug-in-tests.js"; + +module.exports = { + rules: { + "no-debug-in-tests": noDebugInTests, + "ccapi-validate-args": ccAPIValidateArgs, + }, +}; diff --git a/packages/eslint-plugin/src/rules/ccapi-validate-args.ts b/packages/eslint-plugin/src/rules/ccapi-validate-args.ts new file mode 100644 index 000000000000..626513cefc9c --- /dev/null +++ b/packages/eslint-plugin/src/rules/ccapi-validate-args.ts @@ -0,0 +1,155 @@ +import { + AST_NODE_TYPES, + AST_TOKEN_TYPES, + ESLintUtils, + type TSESTree, +} from "@typescript-eslint/utils"; + +const isFixMode = process.argv.some((arg) => arg.startsWith("--fix")); + +export const ccAPIValidateArgs = ESLintUtils.RuleCreator.withoutDocs({ + create(context) { + let currentAPIClassCCName: string | undefined; + let validateArgsImport: string | undefined; + + return { + ImportDeclaration(node) { + if (!!validateArgsImport) return; + if ( + node.source.value === "@zwave-js/transformers" + && node.importKind === "value" + ) { + const importSpecifier = node.specifiers.find((s) => + s.type + === AST_NODE_TYPES.ImportSpecifier + && s.importKind === "value" + && s.imported.name === "validateArgs" + ); + validateArgsImport = importSpecifier?.local.name; + } + }, + ClassDeclaration(node) { + const APIDecorator = node.decorators.find((d) => + d.expression.type === AST_NODE_TYPES.CallExpression + && d.expression.callee.type === AST_NODE_TYPES.Identifier + && d.expression.callee.name === "API" + && d.expression.arguments.length === 1 + && d.expression.arguments[0].type + === AST_NODE_TYPES.MemberExpression + && d.expression.arguments[0].object.type + === AST_NODE_TYPES.Identifier + && d.expression.arguments[0].object.name + === "CommandClasses" + && (d.expression.arguments[0].property.type + === AST_NODE_TYPES.Identifier + || (d.expression.arguments[0].property.type + === AST_NODE_TYPES.Literal + && typeof d.expression.arguments[0].property.value + === "string")) + ); + if (!APIDecorator) return; + + const prop: TSESTree.Literal | TSESTree.Identifier = + (APIDecorator.expression as any).arguments[0].property; + + currentAPIClassCCName = prop.type === AST_NODE_TYPES.Literal + ? prop.value as string + : prop.name; + }, + MethodDefinition(node) { + // Only check methods inside a class decorated with @API + if (!currentAPIClassCCName) return; + // ...that are public + if (node.accessibility !== "public") return; + // ...that have an implementation (so no overload declarations) + if (node.kind !== "method") return; + if (node.value.type !== AST_NODE_TYPES.FunctionExpression) { + return; + } + // ...that have parameters + if (node.value.params.length === 0) return; + // ... and a name + if (node.key.type !== AST_NODE_TYPES.Identifier) return; + + // Ignore some methods + if ( + node.key.name === "supportsCommand" + || node.key.name === "isSetValueOptimistic" + ) { + return; + } + + // Ignore @internal methods + const comments = context.sourceCode.getCommentsBefore(node); + if ( + comments.some((c) => + c.type === AST_TOKEN_TYPES.Block + && c.value.startsWith("*") + && c.value.includes("@internal") + ) + ) { + return; + } + + // Check if the method has an @validateArgs decorator + if ( + node.decorators.some((d) => + d.expression.type === AST_NODE_TYPES.CallExpression + && d.expression.callee.type + === AST_NODE_TYPES.Identifier + && d.expression.callee.name + === (validateArgsImport || "validateArgs") + ) + ) { + return; + } + + // None found, report an error + const lineOfMethod = context.sourceCode.lines[ + node.loc.start.line - 1 + ]; + const indentation = lineOfMethod.slice( + 0, + node.loc.start.column, + ); + + context.report({ + node, + loc: node.key.loc, + messageId: "add-decorator", + fix: isFixMode ? undefined : function*(fixer) { + if (!validateArgsImport) { + validateArgsImport = "validateArgs"; + yield fixer.insertTextBeforeRange( + [0, 0], + `import { validateArgs } from "@zwave-js/transformers";\n`, + ); + } + yield fixer.insertTextBefore( + node, + `@${validateArgsImport}()\n${indentation}`, + ); + }, + }); + }, + "ClassDeclaration:exit"(_node) { + currentAPIClassCCName = undefined; + }, + }; + }, + meta: { + docs: { + description: + "Public CC API methods should have argument validation to catch user errors.", + }, + type: "problem", + // Do not auto-fix these on the CLI + fixable: isFixMode ? undefined : "code", + schema: [], + messages: { + "add-decorator": + "Missing argument validation for public CC API method.", + }, + }, + defaultOptions: [], +}); diff --git a/packages/eslint-plugin/lib/no-debug-in-tests.js b/packages/eslint-plugin/src/rules/no-debug-in-tests.ts similarity index 85% rename from packages/eslint-plugin/lib/no-debug-in-tests.js rename to packages/eslint-plugin/src/rules/no-debug-in-tests.ts index 6a2f690b6ded..202baf172fd0 100644 --- a/packages/eslint-plugin/lib/no-debug-in-tests.js +++ b/packages/eslint-plugin/src/rules/no-debug-in-tests.ts @@ -1,7 +1,6 @@ -// @ts-check -const { AST_NODE_TYPES, ESLintUtils } = require("@typescript-eslint/utils"); -const path = require("node:path"); -const { repoRoot } = require("./utils.js"); +import { AST_NODE_TYPES, ESLintUtils } from "@typescript-eslint/utils"; +import path from "node:path"; +import { repoRoot } from "../utils.js"; const integrationTestDefinitionFiles = new Set( [ @@ -19,9 +18,9 @@ const integrationTestExportNames = new Set([ "integrationTest", ]); -module.exports = ESLintUtils.RuleCreator.withoutDocs({ +export const noDebugInTests = ESLintUtils.RuleCreator.withoutDocs({ create(context) { - const integrationTestMethodNames = new Set(); + const integrationTestMethodNames = new Set(); return { ImportSpecifier(node) { @@ -38,10 +37,6 @@ module.exports = ESLintUtils.RuleCreator.withoutDocs({ )) ) { integrationTestMethodNames.add(node.local.name); - console.log( - "Found integration test method", - node.local.name, - ); } }, Property(node) { diff --git a/packages/eslint-plugin/src/utils.ts b/packages/eslint-plugin/src/utils.ts new file mode 100644 index 000000000000..6a0e0387dbf6 --- /dev/null +++ b/packages/eslint-plugin/src/utils.ts @@ -0,0 +1,5 @@ +import path from "node:path"; + +export const repoRoot = path.normalize( + __dirname.slice(0, __dirname.lastIndexOf(`${path.sep}packages${path.sep}`)), +); diff --git a/packages/eslint-plugin/tsconfig.json b/packages/eslint-plugin/tsconfig.json index 23889e1ece35..f820f68534f5 100644 --- a/packages/eslint-plugin/tsconfig.json +++ b/packages/eslint-plugin/tsconfig.json @@ -1,8 +1,12 @@ { "extends": "@tsconfig/node18/tsconfig.json", "compilerOptions": { - "checkJs": true, - "noEmit": true + "rootDir": "src", + "outDir": "build", + "incremental": true, + "sourceMap": true }, - "include": ["lib/*.js"] + "include": [ + "src/**/*.ts" + ] } diff --git a/packages/zwave-js/src/lib/node/Node.ts b/packages/zwave-js/src/lib/node/Node.ts index af841e738990..f6c37678dfa8 100644 --- a/packages/zwave-js/src/lib/node/Node.ts +++ b/packages/zwave-js/src/lib/node/Node.ts @@ -89,6 +89,7 @@ import { NodeNamingAndLocationCCValues } from "@zwave-js/cc/NodeNamingCC"; import { NotificationCCReport, NotificationCCValues, + getNotificationEnumBehavior, getNotificationStateValueWithEnum, getNotificationValueMetadata, } from "@zwave-js/cc/NotificationCC"; @@ -4367,12 +4368,22 @@ protocol version: ${this.protocolVersion}`; } } if (typeof command.eventParameters === "number") { - // This notification contains an enum value. We set "fake" values for these to distinguish them + // This notification contains an enum value. Depending on how the enum behaves, + // we may need to set "fake" values for these to distinguish them // from states without enum values - const valueWithEnum = getNotificationStateValueWithEnum( - value, - command.eventParameters, - ); + const enumBehavior = valueConfig + ? getNotificationEnumBehavior( + notificationConfig, + valueConfig, + ) + : "extend"; + + const valueWithEnum = enumBehavior === "replace" + ? command.eventParameters + : getNotificationStateValueWithEnum( + value, + command.eventParameters, + ); this.valueDB.setValue(valueId, valueWithEnum); } else { this.valueDB.setValue(valueId, value); diff --git a/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts b/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts index 67f16890cb5a..fb4eb842d8fb 100644 --- a/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts +++ b/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts @@ -7,10 +7,10 @@ import { createMockZWaveRequestFrame } from "@zwave-js/testing"; import { wait } from "alcalzone-shared/async"; import { integrationTest } from "../integrationTestSuite"; -integrationTest( +integrationTest.only( "Notifications with enum event parameters are evaluated correctly", { - // debug: true, + debug: true, nodeCapabilities: { commandClasses: [ @@ -44,8 +44,8 @@ integrationTest( // For the valve operation status variable, the embedded enum replaces its possible states // since there is only one meaningless state, so it doesn't make sense to preserve it // This is different from the "Door state" value which has multiple states AND enums - [0x0100]: "Off / Closed", - [0x0101]: "On / Open", + [0x00]: "Off / Closed", + [0x01]: "On / Open", }); // Send notifications to the node @@ -64,7 +64,7 @@ integrationTest( await wait(100); let value = node.getValue(valveOperationStatusId); - t.is(value, 0x0100); + t.is(value, 0x00); cc = new NotificationCCReport(mockNode.host, { nodeId: mockController.host.ownNodeId, @@ -80,13 +80,13 @@ integrationTest( await wait(100); value = node.getValue(valveOperationStatusId); - t.is(value, 0x0101); + t.is(value, 0x01); }, }, ); integrationTest( - "Notification types multiple states and optional enums merge/extend states for all of them", + "Notification types with multiple states and optional enums merge/extend states for all of them", { // debug: true, @@ -104,16 +104,15 @@ integrationTest( ], }, - testBody: async (t, driver, node, _mockController, _mockNode) => { + testBody: async (t, driver, node, mockController, mockNode) => { await node.commandClasses.Notification.getSupportedEvents(0x06); + const doorStateValueId = NotificationCCValues.notificationVariable( + "Access Control", + "Door state", + ).id; const states = ( - node.getValueMetadata( - NotificationCCValues.notificationVariable( - "Access Control", - "Door state", - ).id, - ) as ValueMetadataNumeric + node.getValueMetadata(doorStateValueId) as ValueMetadataNumeric ).states; t.deepEqual(states, { [0x16]: "Window/door is open", @@ -123,6 +122,70 @@ integrationTest( [0x1600]: "Window/door is open in regular position", [0x1601]: "Window/door is open in tilt position", }); + + // Send notifications to the node + let cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x16, + eventParameters: Buffer.from([0x00]), // open in regular position + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + // wait a bit for the value to be updated + await wait(100); + + let value = node.getValue(doorStateValueId); + t.is(value, 0x1600); + + cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x16, + eventParameters: Buffer.from([0x01]), // open in tilt position + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + await wait(100); + + value = node.getValue(doorStateValueId); + t.is(value, 0x1601); + + cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x16, // open + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + await wait(100); + + value = node.getValue(doorStateValueId); + t.is(value, 0x16); + + cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x17, // closed + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + await wait(100); + + value = node.getValue(doorStateValueId); + t.is(value, 0x17); }, }, );