Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic eslint fixes #26

Closed
wants to merge 4 commits into from
Closed

Basic eslint fixes #26

wants to merge 4 commits into from

Conversation

tony
Copy link
Contributor

@tony tony commented Dec 4, 2021

Lint fixes on top of #25

Fixes:

❯ npm run lint

> @zaproxy/actions-common-scans@0.2.0 lint /home/t/work/zaproxy/actions-common/packages/scans
> eslint src/*.js

/home/t/work/zaproxy/actions-common/packages/scans/src/action-helper.js
   59:22  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
   78:22  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
   88:22  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  127:35  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  127:86  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  129:40  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  160:23  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
  167:19  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins

/home/t/work/zaproxy/actions-common/packages/scans/src/index.js
   1:7  error  'core' is assigned a value but never used     no-unused-vars
   2:7  error  'exec' is assigned a value but never used     no-unused-vars
   5:7  error  '_' is assigned a value but never used        no-unused-vars
  36:9  error  'context' is assigned a value but never used  no-unused-vars

✖ 12 problems (12 errors, 0 warnings)

See also: https://github.com/eslint/eslint/blob/v8.4.0/CHANGELOG.md

Signed-off-by: Tony Narlock <tony@git-pull.com>
Signed-off-by: Tony Narlock <tony@git-pull.com>
Signed-off-by: Tony Narlock <tony@git-pull.com>
Signed-off-by: Tony Narlock <tony@git-pull.com>
@@ -14,6 +14,10 @@ function createReadStreamSafe(filename, options) {
});
}

function hasOwnProperty(obj, key) {
return Object.prototype.hasOwnProperty.call(obj, key);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Via no-prototype-builtins via eslint:recommended

MDN: Object.prototype.hasOwnProperty()

Personally I'd prefer optional chaining, e.g.:

diff --git a/packages/scans/src/action-helper.js b/packages/scans/src/action-helper.js
index 49cbc8a..c3a6c8a 100644
--- a/packages/scans/src/action-helper.js
+++ b/packages/scans/src/action-helper.js
@@ -60,7 +60,7 @@ let actionHelper = {

         sites.forEach((site => {
             msg = msg + `${BULLET} Site: [${site["@name"]}](${site["@name"]}) ${NXT_LINE}`;
-            if (hasOwnProperty(site, 'alerts')) {
+            if (site?.alerts) {
                 if (site.alerts.length !== 0) {
                     msg = `${msg} ${TAB} **New Alerts** ${NXT_LINE}`;
                     site.alerts.forEach((alert) => {

If we intend on ever doing a dist/ release with this, ncc raised an error:

./node_modules/.bin/ncc build
ncc: Version 0.21.1
ncc: Compiling file index.js
Error: Module parse failed: Unexpected token (63:21)
File was processed with these loaders:
 * ./node_modules/@zeit/ncc/dist/ncc/loaders/empty-loader.js
 * ./node_modules/@zeit/ncc/dist/ncc/loaders/relocate-loader.js
 * ./node_modules/@zeit/ncc/dist/ncc/loaders/shebang-loader.js
You may need an additional loader to handle the result of these loaders.
|         sites.forEach((site => {
|             msg = msg + `${BULLET} Site: [${site["@name"]}](${site["@name"]}) ${NXT_LINE}`;
>             if (site?.alerts) {
|                 if (site.alerts.length !== 0) {
|                     msg = `${msg} ${TAB} **New Alerts** ${NXT_LINE}`;
    at evalmachine.<anonymous>:1:1416387
    at eval (eval at create (evalmachine.<anonymous>:1:274053), <anonymous>:13:1)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

If we update ncc from 0.21.0 to 0.33.0 it fixes it:

npm uninstall @zeit/ncc && npm install --save-dev @vercel/ncc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #27

@tony
Copy link
Contributor Author

tony commented Feb 22, 2022

Same as #25 and there's indication that xo may be used: Back in #29 (comment). I think if such a thing is done it should be a new PR

@tony tony closed this Feb 22, 2022
@tony tony deleted the basic-eslint-fixes branch February 22, 2022 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant