Skip to content

Commit

Permalink
Re-enable cpplint & eslint for Linux platform actions (#1057)
Browse files Browse the repository at this point in the history
This patch migrates `ESLint` configuration, `.eslintrc.yml` to [the new flat config format](https://eslint.org/docs/latest/use/configure/migration-guide) by creating `eslint.config.mjs`, meanwhile, re-enables [cpplint](https://github.com/cpplint/cpplint) for Linux platform actions, including:

1. Config `eslint` using recommended configs for `*.js` files in`eslint.config.mjs`.
2. Config `eslint-plugin-prettier` using recommended configs for `*.js` files in `eslint.config.mjs`.
3. Config `@typescript-eslint/eslint-plugin` using recommended configs for `*.d.ts` files in `eslint.config.mjs`.
4. Re-enable [`cpplint.py`](https://github.com/cpplint/cpplint/blob/develop/cpplint.py) for `*.{hpp,cpp}` files.
5. Fix errors/warnings reported by linters above.
6. Add `npm run lint` as addition step for actions, `.github/workflows/linux-build-and-test-compatibility.yml` and `.github/workflows/linux-build-and-test.yml`.
7. Remove original `.eslintrc.yml`.

Fix: #1056
  • Loading branch information
minggangw authored Mar 5, 2025
1 parent 120fcd0 commit 1d8b40c
Show file tree
Hide file tree
Showing 67 changed files with 152 additions and 242 deletions.
25 changes: 0 additions & 25 deletions .eslintrc.yml

This file was deleted.

1 change: 1 addition & 0 deletions .github/workflows/linux-build-and-test-compatibility.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ jobs:
run: |
source /opt/ros/${{ matrix.ros_distribution }}/setup.bash
npm i
npm run lint
npm test
1 change: 1 addition & 0 deletions .github/workflows/linux-build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,5 @@ jobs:
run: |
source /opt/ros/${{ needs.identify-ros-distro.outputs.distro }}/setup.bash
npm i
npm run lint
npm test
65 changes: 65 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import typescriptEslint from "@typescript-eslint/eslint-plugin";
import prettier from "eslint-plugin-prettier";
import globals from "globals";
import tsParser from "@typescript-eslint/parser";
import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended';
import js from "@eslint/js";

export default [
{
ignores: [
"eslint.config.mjs",
"types/interfaces.d.ts",
"test/types/index.test-d.ts",
"**/generated/",
"**/scripts/",
"**/benchmark/",
"**/docs/",
"**/electron_demo/",
],
},
{
...js.configs.recommended,
files: ["lib/**/*.js", "index.js"],
},
{
rules: {
'no-dupe-class-members': 'off',
},
},
{
plugins: {
"@typescript-eslint": typescriptEslint,
},
languageOptions: {
globals: {
...globals.node,
},
parser: tsParser,
ecmaVersion: "latest",
sourceType: "module",
},
files: ['types/*.d.ts'],
rules: {
...typescriptEslint.configs.recommended.rules,
"@typescript-eslint/no-explicit-any": "off",
},
},
{
plugins: {
prettier,
},
languageOptions: {
globals: {
...globals.node,
},
ecmaVersion: "latest",
sourceType: "commonjs",
},
files: ["lib/**/*.js", "rosidl_parser/**/*.js", "rosidl_gen/**/*.js",
"rostsd_gen/**/*.js", "test/**/*.js", "example/**/*.js", "index.js"],
rules: {
...eslintPluginPrettierRecommended.rules,
},
}
];
2 changes: 0 additions & 2 deletions example/publisher-content-filter-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable camelcase */

const rclnodejs = require('../index.js');

async function main() {
Expand Down
2 changes: 0 additions & 2 deletions example/publisher-message-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable camelcase */

const rclnodejs = require('../index.js');

rclnodejs
Expand Down
2 changes: 0 additions & 2 deletions example/publisher-multiarray-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable camelcase */

const rclnodejs = require('../index.js');

rclnodejs
Expand Down
2 changes: 0 additions & 2 deletions example/publisher-raw-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable camelcase */

const rclnodejs = require('../index.js');

rclnodejs
Expand Down
2 changes: 1 addition & 1 deletion example/subscription-multiarray-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ rclnodejs
const weightStride = dim[1].stride;
const channel = dim[2].size;
const channelStride = dim[2].stride;
// eslint-disable-next-line

const offset = multiArray.layout.data_offset;

for (let i = 0; i < height; i++) {
Expand Down
2 changes: 0 additions & 2 deletions example/subscription-raw-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable camelcase */

const rclnodejs = require('../index.js');

rclnodejs.init().then(() => {
Expand Down
2 changes: 1 addition & 1 deletion lib/action/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class ActionClient extends Entity {
goalHandle.status = status;

// Remove done handles from the list
// eslint-disable-next-line max-depth

if (
status === ActionInterfaces.GoalStatus.STATUS_SUCCEEDED ||
status === ActionInterfaces.GoalStatus.STATUS_CANCELED ||
Expand Down
2 changes: 0 additions & 2 deletions lib/clock_type.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable camelcase */

/**
* Enum for ClockType
* @readonly
Expand Down
2 changes: 1 addition & 1 deletion lib/distro.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const DistroUtils = {
return process.env.ROS_DISTRO;
}

return [...DistroNameIdMap].find(([key, val]) => val == distroId)[0];
return [...DistroNameIdMap].find(([, val]) => val == distroId)[0];
},

getKnownDistroNames: function () {
Expand Down
3 changes: 1 addition & 2 deletions lib/interface_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ let interfaceLoader = {
// Suppose the name is a package, and traverse the path to collect the IDL files.
let packagePath = path.join(generator.generatedRoot, name);

// eslint-disable-next-line
let interfaces = fs.readdirSync(packagePath);
if (interfaces.length > 0) {
return this.loadInterfaceByPath(packagePath, interfaces);
Expand Down Expand Up @@ -108,7 +107,7 @@ let interfaceLoader = {
packageName,
packageName + '__' + type + '__' + messageName + '.js'
);
// eslint-disable-next-line

if (fs.existsSync(filePath)) {
return require(filePath);
}
Expand Down
5 changes: 0 additions & 5 deletions lib/lifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,6 @@ class LifecycleNode extends Node {
* @throws {Error} If transition is invalid for the current state.
*/
shutdown(callbackReturnValue) {
let state = this.currentState;

return this._changeState(SHUTDOWN_TRANSITION_LABEL, callbackReturnValue);
}

Expand All @@ -603,7 +601,6 @@ class LifecycleNode extends Node {
_onGetState(request, response) {
let result = response.template;

// eslint-disable-next-line camelcase
result.current_state = this.currentState;

response.send(result);
Expand All @@ -619,7 +616,6 @@ class LifecycleNode extends Node {
_onGetAvailableStates(request, response) {
let result = response.template;

// eslint-disable-next-line camelcase
result.available_states = this.availableStates;

response.send(result);
Expand All @@ -634,7 +630,6 @@ class LifecycleNode extends Node {
_onGetAvailableTransitions(request, response) {
let result = response.template;

// eslint-disable-next-line camelcase
result.available_transitions = this.availableTransitions;

response.send(result);
Expand Down
4 changes: 2 additions & 2 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -1549,7 +1549,7 @@ class Node extends rclnodejs.ShadowNode {
// detect invalid parameter
try {
parameter.validate();
} catch (e) {
} catch {
return {
successful: false,
reason: `Invalid ${parameter.name}`,
Expand Down Expand Up @@ -1577,7 +1577,7 @@ class Node extends rclnodejs.ShadowNode {
if (parameter.type != ParameterType.PARAMETER_NOT_SET) {
try {
descriptor.validateParameter(parameter);
} catch (e) {
} catch {
return {
successful: false,
reason: `Parameter ${parameter.name} does not readonly`,
Expand Down
14 changes: 6 additions & 8 deletions lib/parameter.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

'use strict';

/* eslint-disable camelcase */

const IsClose = require('is-close');

/**
Expand Down Expand Up @@ -273,7 +271,6 @@ class ParameterDescriptor {
static fromParameter(parameter) {
const name = parameter.name;
const type = parameter.type;
const value = parameter.value;
return new ParameterDescriptor(name, type, 'Created from parameter.');
}

Expand Down Expand Up @@ -386,7 +383,7 @@ class ParameterDescriptor {
return;
}
if (!(range instanceof Range)) {
throw TypeException('Expected instance of Range.');
throw TypeError('Expected instance of Range.');
}
if (!range.isValidType(this.type)) {
throw TypeError('Incompatible Range');
Expand Down Expand Up @@ -442,14 +439,14 @@ class ParameterDescriptor {
// ensure parameter is valid
try {
parameter.validate();
} catch (e) {
} catch {
throw new TypeError('Parameter is invalid');
}

// ensure this descriptor is valid
try {
this.validate();
} catch (e) {
} catch {
throw new Error('Descriptor is invalid.');
}

Expand Down Expand Up @@ -566,6 +563,7 @@ class Range {
* @param {ParameterType} parameterType - The parameter type to test.
* @return {boolean} - True if parameterType is compatible; otherwise return false.
*/
// eslint-disable-next-line no-unused-vars
isValidType(parameterType) {
return false;
}
Expand Down Expand Up @@ -701,6 +699,7 @@ class IntegerRange extends Range {
* @param {any} value - The value to infer it's ParameterType
* @returns {ParameterType} - The ParameterType that best scribes the value.
*/
// eslint-disable-next-line no-unused-vars
function parameterTypeFromValue(value) {
if (!value) return ParameterType.PARAMETER_NOT_SET;
if (typeof value === 'boolean') return ParameterType.PARAMETER_BOOL;
Expand Down Expand Up @@ -777,8 +776,7 @@ function validValue(value, type) {
case ParameterType.PARAMETER_INTEGER_ARRAY:
case ParameterType.PARAMETER_DOUBLE_ARRAY:
case ParameterType.PARAMETER_STRING_ARRAY:
const values = value;
result = _validArray(values, type);
result = _validArray(value, type);
break;
default:
result = false;
Expand Down
2 changes: 0 additions & 2 deletions lib/parameter_service.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable max-depth */
// Copyright (c) 2020 Wayne Parrott. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -15,7 +14,6 @@

'use strict';

const rclnodejs = require('bindings')('rclnodejs');
const { Parameter, PARAMETER_SEPARATOR } = require('./parameter.js');

/**
Expand Down
2 changes: 0 additions & 2 deletions lib/qos.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

'use strict';

/* eslint-disable */

/**
* Enum for HistoryPolicy
* @readonly
Expand Down
1 change: 0 additions & 1 deletion lib/time_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
const rclnodejs = require('bindings')('rclnodejs');
const { Clock, ROSClock } = require('./clock.js');
const { ClockType } = Clock;
const Node = require('./node.js');
const { Parameter, ParameterType } = require('./parameter.js');
const Time = require('./time.js');

Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"postinstall": "npm run generate-messages",
"docs": "cd docs && make",
"test": "node --expose-gc ./scripts/run_test.js && npx tsd",
"lint": "eslint --max-warnings=0 --ext js,ts index.js types scripts lib example rosidl_gen rosidl_parser test benchmark/rclnodejs && node ./scripts/cpplint.js",
"lint": "eslint && node ./scripts/cpplint.js",
"format": "clang-format -i -style=file ./src/*.cpp ./src/*.hpp && prettier --write \"{lib,rosidl_gen,rostsd_gen,rosidl_parser,types,example,test,scripts,benchmark}/**/*.{js,md,ts}\" ./*.{js,md,ts}",
"prepare": "husky"
},
Expand All @@ -43,17 +43,17 @@
"url": "git+https://github.com/RobotWebTools/rclnodejs.git"
},
"devDependencies": {
"@babel/eslint-parser": "^7.25.9",
"@types/node": "^22.13.5",
"@eslint/js": "^10.0.0",
"@types/node": "^22.13.9",
"@typescript-eslint/eslint-plugin": "^8.18.0",
"@typescript-eslint/parser": "^8.18.0",
"babel-eslint": "^10.1.0",
"clang-format": "^1.8.0",
"commander": "^13.1.0",
"deep-equal": "^2.2.3",
"eslint": "^9.16.0",
"eslint-config-prettier": "^10.0.1",
"eslint-config-prettier": "^10.0.2",
"eslint-plugin-prettier": "^5.2.1",
"globals": "^16.0.0",
"husky": "^9.1.7",
"jsdoc": "^4.0.4",
"lint-staged": "^15.2.10",
Expand Down
1 change: 0 additions & 1 deletion rosidl_gen/action_msgs.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

'use strict';

/* eslint-disable camelcase */
const GOAL_ID_FIELD = {
name: 'goal_id',
type: {
Expand Down
1 change: 0 additions & 1 deletion rosidl_gen/filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ const RosPackageFilters = {

if (!fs.existsSync(blocklistPath)) return;

// eslint-disable-next-line
let blocklistData = JSON.parse(fs.readFileSync(blocklistPath, 'utf8'));

let filters = blocklistData.map((pkgFilterData) => {
Expand Down
Loading

0 comments on commit 1d8b40c

Please sign in to comment.