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

test: a try to add linters to E2E tests #1067

Closed
wants to merge 11 commits into from
Closed

Conversation

EvgeniiZaitsevCW
Copy link
Contributor

  1. The ESLint and Solhint linters have been added and configured in folder e2e to check and auto-format TypeScript and JavaScript files.
  2. The ESLint tool has been extended with the ESLint Stylistic plugin to check and correct formatting issues (indents, spaces, etc.).
  3. The existing Prettier tool has been extended to auto-format the Solidity code.
  4. The .prettierignore file has been added to make Prettier ignore auto-generated folders (artifacts, cache, typechain-types).
  5. The errors and warnings found by the new linters have been fixed in the TypeScript, JavaScript and Solidity files.
  6. The e2e-lint option in justfile has been corrected to run the new linters instead of Prettier.
  7. The option to run Prettier has been renamed to e2e-prettier in justfile.
  8. Some unused code related to changing of the RPC provider has been removed from the e2e/test/helpers/rpc.ts file. That code is currently used in the e2e-contracts folder.
  9. IMPORTANT! Prettier not always gives the good results of code formatting. So if you run just e2e-prettier you'll still find some possible changes in the code, but they are not applied intentionally due to the existing version seems more readable than that Prettier provides. It is also the reason why Prettier has not been used for check the formatting (e.g. for Solidity files through solhint-plugin-prettier). Linters are more suitable here. Unfortunately, no good supported formatting linter for Solidity was found. Solhint supported formatting checks up to version 3, but then dropped that (see this release note here). So it is expected that Prettier will be used to auto-format Solidity code, but then the results will be checked and inappropriate changes will be rolled back.
  10. An example of a well-known issue with Prettier when you apply it is applied to Solidity is formatting intentional multi-line entities into a single line. E.g. the following code:
    event Add(
        address indexed account,
        uint256 amount
    );
    will be changed as:
    event Add(address indexed account, uint256 amount);
    A workaround to prevent that is use a comment on the first line, like:
    event Add(
        address indexed account, //
        uint256 amount
    );
    But it looks a bit awkward.

@EvgeniiZaitsevCW EvgeniiZaitsevCW requested a review from a team as a code owner June 11, 2024 17:51
Copy link

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]

4

🧪 Relevant tests

No

🔒 Security concerns

No

⚡ Key issues to review

Logging and Monitoring:
The PR does not show explicit additions or modifications related to logging or monitoring of background operations. It's important to ensure that all significant actions, especially those running in the background, are properly logged and monitored for better maintainability and debugging.

Async Task Handling:
The PR does not clearly indicate if all async tasks are confined to the tokio runtime as per the team's decision. Ensure that any blocking tasks are handled in separate threads to avoid performance bottlenecks.

Code Consistency:
There are several instances where the code style and formatting could be improved for better consistency. For example, the use of let and const in TypeScript files should be consistent according to the scope and reassignment properties of variables.

Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Possible bug
Add a null check for the environment variable before using it

Consider checking the environment variable BLOCK_MODE for null or undefined before using
it in the defineBlockMiningIntervalInMs function to avoid runtime errors if the
environment variable is not set.

e2e/hardhat.config.ts [36]

-interval: defineBlockMiningIntervalInMs(process.env.BLOCK_MODE),
+interval: process.env.BLOCK_MODE ? defineBlockMiningIntervalInMs(process.env.BLOCK_MODE) : defaultInterval,
 
Suggestion importance[1-10]: 9

Why: This suggestion addresses a potential runtime error by adding a null check for the environment variable, which is crucial for preventing unexpected crashes.

9
Add error handling or default value for process.env.BLOCK_MODE to avoid potential runtime errors

Consider handling the case where process.env.BLOCK_MODE might be undefined or not a valid
key for defineBlockMiningIntervalInMs, which could lead to runtime errors. Implement a
fallback or default value, or validate the environment variable before usage.

e2e/test/helpers/network.ts [46]

-return defineBlockMiningIntervalInMs(process.env.BLOCK_MODE);
+const blockMode = process.env.BLOCK_MODE || 'defaultMode';
+return defineBlockMiningIntervalInMs(blockMode);
 
Suggestion importance[1-10]: 9

Why: This suggestion addresses a potential runtime error by adding a default value for process.env.BLOCK_MODE. This is crucial for ensuring the robustness of the code.

9
Performance
Replace the infinite loop with a finite loop to prevent potential resource exhaustion

Replace the infinite loop with a more controlled loop mechanism to prevent potential high
CPU usage or infinite execution which can lead to application crashes or unresponsiveness.

e2e/perf/load-test.ts [8]

-while (true) {
+for (let i = 0; i < maxIterations; i++) {
 
Suggestion importance[1-10]: 8

Why: Replacing the infinite loop with a finite loop improves performance and prevents potential resource exhaustion, which is a significant improvement for stability.

8
Possible issue
Add a maximum number of retries to the polling loop to prevent potential infinite loops

The pollForTransactions function uses a while loop that could potentially run indefinitely
if the conditions are not met. Consider implementing a maximum number of retries or a more
robust exit condition.

e2e/test/helpers/rpc.ts [359-360]

-while (1) {
+let attempts = 0;
+const maxAttempts = 100;
+while (attempts < maxAttempts) {
     const requestTimeInMs = Date.now();
     ...
+    attempts++;
 }
 
Suggestion importance[1-10]: 8

Why: This suggestion addresses a potential infinite loop issue, which is important for preventing the application from hanging indefinitely.

8
Add error handling for the sendExpect function to manage exceptions gracefully

Ensure that the sendExpect function handles exceptions or errors gracefully, especially
when used in a test environment where the function might throw an error or return an
unexpected result.

e2e/test/automine/e2e-json-rpc.test.ts [49]

-const client = await sendExpect("web3_clientVersion");
+let client;
+try {
+  client = await sendExpect("web3_clientVersion");
+} catch (error) {
+  console.error("Failed to get client version:", error);
+}
 
Suggestion importance[1-10]: 7

Why: Adding error handling for the sendExpect function enhances robustness, especially in a test environment, but it is less critical than runtime checks or performance improvements.

7
Add error handling for the sendRawTransactions function to manage transaction failures

Consider adding error handling for the sendRawTransactions function to handle cases where
the transaction might fail, ensuring that the application can gracefully handle such
scenarios.

e2e/test/automine/e2e-tx-parallel-contract.test.ts [87]

-const hashes = await sendRawTransactions(signedTxs);
+let hashes;
+try {
+  hashes = await sendRawTransactions(signedTxs);
+} catch (error) {
+  console.error("Error sending transactions:", error);
+}
 
Suggestion importance[1-10]: 7

Why: Adding error handling for transaction sending improves the application's ability to manage failures gracefully, enhancing reliability.

7
Best practice
Improve type accuracy for transaction index

Consider using a more specific type assertion for receipt.transactionIndex instead of
number, as it should be a hexadecimal string according to Ethereum JSON-RPC
specifications.

e2e/test/automine/e2e-tx-serial-contract.test.ts [82]

-const actualTransactionIndex = receipt.transactionIndex as number;
+const actualTransactionIndex = receipt.transactionIndex as string;
 
Suggestion importance[1-10]: 8

Why: This suggestion improves type accuracy by aligning with Ethereum JSON-RPC specifications, which enhances code reliability and correctness.

8
Use stricter equality checks for transaction status

Use the toStrictEqual method for a stricter equality check that ensures the type and value
are the same, which is more appropriate for checking blockchain transaction properties.

e2e/test/automine/e2e-tx-serial-contract.test.ts [90]

-expect(receipt.status).eq(STATUS_SUCCESS, "receipt.status");
+expect(receipt.status).toStrictEqual(STATUS_SUCCESS, "receipt.status");
 
Suggestion importance[1-10]: 6

Why: Using toStrictEqual ensures both type and value are checked, which is more appropriate for verifying blockchain transaction properties, enhancing test accuracy.

6
Ensure consistent use of async/await for reliability and readability

Use async/await consistently in the test hooks and cases to ensure that all asynchronous
operations are handled properly, improving test reliability and readability.

e2e/test/issues/external/e2e-ebm-pending-transactions.ts [10]

 before(async () => {
-    expect(currentBlockMode()).eq(BlockMode.External, "Wrong block mining mode is used");
+    await expect(currentBlockMode()).eq(BlockMode.External, "Block mode should be External");
 });
 
Suggestion importance[1-10]: 3

Why: The suggestion to use await with expect is incorrect because expect is not an asynchronous function. This would introduce a syntax error, making the suggestion invalid.

3
Enhancement
Dynamically set the expected transaction index

Instead of hardcoding the expected transaction index, consider retrieving it dynamically
from the transaction receipt to ensure the test remains valid even if the transaction
order changes.

e2e/test/automine/e2e-tx-serial-contract.test.ts [81]

-const expectedTransactionIndex = "0x0";
+const expectedTransactionIndex = receipt.transactionIndex;
 
Suggestion importance[1-10]: 7

Why: Dynamically retrieving the transaction index ensures the test remains valid even if the transaction order changes, improving test robustness.

7
Replace process.exit(1) with throwing an error for better error handling

The error handling for the ETHERJS.on("debug", log) event listener setup uses
process.exit(1), which can be abrupt. Consider throwing an error or using a more graceful
error handling strategy.

e2e/test/helpers/rpc.ts [81-84]

 ETHERJS.on("debug", log).catch((error) => {
     console.error("Registering listener for a debug event of the RPC failed:", error);
-    process.exit(1);
+    throw new Error("Failed to register debug event listener.");
 });
 
Suggestion importance[1-10]: 7

Why: This suggestion improves error handling by avoiding abrupt termination of the process, which is beneficial for debugging and maintaining application stability.

7
Review and potentially adjust the batchMaxCount for appropriate batch processing

The batchMaxCount option is set to 1, which disables batch processing. If this is
intentional, consider adding a comment explaining the reason. If batch processing should
be enabled, adjust the batchMaxCount to a higher value.

e2e/test/helpers/rpc.ts [57]

-batchMaxCount: 1, // Do not unite the request in batches
+batchMaxCount: 10, // Adjust batch size as needed
 
Suggestion importance[1-10]: 5

Why: The suggestion is valid but not critical. It addresses a potential optimization or clarification but does not fix a bug or critical issue.

5
Maintainability
Refine the assertion message for clarity and direct relevance

Ensure that the assertion message in the before hook is concise and directly related to
the test context to improve clarity and maintainability of the test suite.

e2e/test/issues/external/e2e-ebm-pending-transactions.ts [10]

-expect(currentBlockMode()).eq(BlockMode.External, "Wrong block mining mode is used");
+expect(currentBlockMode()).eq(BlockMode.External, "Block mode should be External");
 
Suggestion importance[1-10]: 7

Why: The refined assertion message is clearer and more directly related to the test context, which improves the maintainability and readability of the test suite. However, the existing message is already understandable.

7
Improve readability and consistency by destructuring imports

Consider destructuring the imports from "ethers" to maintain consistency and improve
readability. This will align with how other modules are imported and make the code
cleaner.

e2e/test/issues/external/e2e-ebm-pending-transactions.ts [2]

-import { TransactionResponse, keccak256 } from "ethers";
+import { ethers } from "ethers";
+const { TransactionResponse, keccak256 } = ethers;
 
Suggestion importance[1-10]: 5

Why: While destructuring imports can improve readability and consistency, the current import statement is already clear and concise. The improvement is minor and does not significantly impact the maintainability of the code.

5

// ----------------- TypeScript only files config ------------------ //

// TypeScript recommended config
...tseslint.configs.recommended.map((config) => ({

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Prefer '_.map' over the native function.

The issue identified by ESLint is that the native Array.prototype.map function is being used instead of the _.map function from the Lodash library. Lodash's _.map function is often preferred in some codebases for consistency, additional functionality, or performance reasons.

Here's the code suggestion to fix the issue by replacing the native map function with Lodash's _.map:

Suggested change
...tseslint.configs.recommended.map((config) => ({
..._.map(tseslint.configs.recommended, (config) => ({

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");
const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: ES2015 block-scoped variables are forbidden.

The issue reported by ESLint is that const is being used in a block-scoped manner, which is forbidden by the linter's configuration. This typically means that the linter is configured to disallow ES2015 features like const and let, likely to maintain compatibility with older JavaScript environments that do not support these features.

To fix this issue, you can replace const with var, which is function-scoped and compatible with older JavaScript versions.

Suggested change
const tseslint = require("typescript-eslint");
var tseslint = require("typescript-eslint");

This comment was generated by an experimental AI tool.

quotes: "double",
semi: true,
jsx: false,
arrowParens: true,

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: You have a misspelled word: Parens on Identifier

The issue identified by the ESLint linter is that the word "Parens" is misspelled in the configuration object. The correct term should be "parens" (all lowercase). ESLint configuration properties are case-sensitive, and using an incorrect property name will result in the configuration not being applied as intended.

Here's the corrected line of code:

Suggested change
arrowParens: true,
arrowparens: true,

This comment was generated by an experimental AI tool.

function changeErrorToWarn(configObj) {
for (const key in configObj) {
if (configObj[key] === "error") {
configObj[key] = "warn";

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Security issue: Generic Object Injection Sink

The issue described as a "Generic Object Injection Sink" refers to the potential vulnerability where an attacker could manipulate the properties of an object in an unintended way. This can occur if an object is modified based on user input or other untrusted sources, leading to security risks such as prototype pollution.

In the provided code, the line configObj[key] = "warn"; directly assigns a value to a property of configObj. If configObj is derived from user input or other untrusted sources, this could allow an attacker to inject malicious properties into the object.

To fix this issue, you should ensure that the key being modified is a legitimate property of the object and not something inherited from the prototype chain. This can be done using the hasOwnProperty method.

Here is the code suggestion to fix the issue:

Suggested change
configObj[key] = "warn";
if (configObj.hasOwnProperty(key)) configObj[key] = "warn";

This change ensures that only properties that are directly on the configObj (and not inherited) are modified, mitigating the risk of object injection.


This comment was generated by an experimental AI tool.

if (configObj[key] === "error") {
configObj[key] = "warn";
} else if (typeof configObj[key] === "object") {
const nestedConfigObj = configObj[key];

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Security issue: Variable Assigned to Object Injection Sink

The issue identified by the ESLint linter is related to a potential object injection vulnerability. This occurs when user-controlled input is used to access or modify properties on an object, which can lead to unexpected behavior or security risks if the input is not properly validated.

In the provided code, the line const nestedConfigObj = configObj[key]; directly assigns a nested object based on a key from the configObj. If the configObj is constructed from untrusted input, an attacker could manipulate the key to inject malicious properties or values.

To mitigate this, you should ensure that the key is a valid property of the configObj and not an inherited property or prototype pollution. Using Object.prototype.hasOwnProperty.call is a safe way to check if the object has the property as its own.

Here's the single line change to fix the issue:

Suggested change
const nestedConfigObj = configObj[key];
if (Object.prototype.hasOwnProperty.call(configObj, key) && typeof configObj[key] === "object") {

This change ensures that the key is a direct property of configObj and not an inherited one, thus mitigating the object injection vulnerability.


This comment was generated by an experimental AI tool.

return configObj;
}

const stylisticRecommendedConfig = stylistic.configs.customize({

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: ES5 trailing commas in array/object literals are forbidden.

The issue described by ESLint is that trailing commas are not allowed in ES5, which is why the linter flagged the line where the object literal is being customized. Specifically, the trailing comma after the last property commaDangle: "always-multiline" is causing the error.

To fix this issue, you need to remove the trailing comma from the object literal. Here is the corrected line:

Suggested change
const stylisticRecommendedConfig = stylistic.configs.customize({
commaDangle: "always-multiline"

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");
const pluginJs = require("@eslint/js");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: "@eslint/js" is extraneous.

The issue described by ESLint indicates that the module "@eslint/js" is not necessary for the code to function correctly. It is considered extraneous, meaning it can be removed without affecting the script's behavior. This could be due to the module not being used anywhere else in the code.

To fix this issue, you should remove the line where "@eslint/js" is required. Here is the suggested change:

Suggested change
const pluginJs = require("@eslint/js");
// const pluginJs = require("@eslint/js");

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");
const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Require statement not part of import statement.

The issue reported by ESLint is that the require statement is not being used as an import statement. In modern JavaScript, especially in ES6 and later, it is recommended to use import statements instead of require statements for module imports. This ensures consistency and takes advantage of the static analysis benefits provided by ES6 modules.

To fix this issue, you should replace the require statement with an import statement.

Here is the corrected line:

Suggested change
const tseslint = require("typescript-eslint");
import * as tseslint from "typescript-eslint";

This comment was generated by an experimental AI tool.

const INDENT_SIZE = 4;

function changeErrorToWarn(configObj) {
for (const key in configObj) {

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: ES2015 block-scoped variables are forbidden.

The issue here is that the ESLint linter has detected the use of a const declaration in a for...in loop. const is block-scoped, which means it is not suitable for use in a loop where the variable is intended to be re-assigned. Instead, a let declaration should be used for the loop variable to allow for proper iteration.

Here’s the single line change to fix the issue:

Suggested change
for (const key in configObj) {
for (let key in configObj) {

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");
const pluginJs = require("@eslint/js");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Require statement not part of import statement.

The issue identified by the ESLint linter is that the require statement is being used instead of an import statement. In modern JavaScript (ES6 and beyond), it is recommended to use import statements for module imports instead of require statements, which are part of CommonJS and are generally used in Node.js environments.

To fix this issue, you should replace the require statement with an import statement. Here's the single line change required:

Suggested change
const pluginJs = require("@eslint/js");
import pluginJs from "@eslint/js";

You would apply similar changes to the other require statements in the code fragment to ensure consistency and adherence to modern JavaScript standards.


This comment was generated by an experimental AI tool.

const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");
const stylistic = require("@stylistic/eslint-plugin");
const sort = require("eslint-plugin-sort");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: "eslint-plugin-sort" is not published.

The issue is that the eslint-plugin-sort package is not a published npm package, which means it cannot be installed or used in the project. This will cause an error when trying to require it.

To fix this issue, you should remove the line that imports the non-existent eslint-plugin-sort package.

Here's the single line change to fix the issue:

Suggested change
const sort = require("eslint-plugin-sort");
// const sort = require("eslint-plugin-sort");

This comment was generated by an experimental AI tool.

for (const key in configObj) {
if (configObj[key] === "error") {
configObj[key] = "warn";
} else if (typeof configObj[key] === "object") {

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Prefer '_.isObject' over 'typeof' comparison.

The issue identified by ESLint is that using typeof configObj[key] === "object" is not a reliable way to check if a value is an object. This is because typeof null also returns "object", which can lead to unexpected behavior. Instead, using a utility function like _.isObject from the Lodash library is recommended, as it provides a more accurate check.

Here's the code suggestion to fix the issue using Lodash's _.isObject function:

Suggested change
} else if (typeof configObj[key] === "object") {
} else if (_.isObject(configObj[key])) {

This comment was generated by an experimental AI tool.

const nestedConfigObj = configObj[key];
for (const key in nestedConfigObj) {
if (nestedConfigObj[key] === "error") {
nestedConfigObj[key] = "warn";

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Security issue: Generic Object Injection Sink

The issue identified by ESLint as a "Generic Object Injection Sink" is related to the potential risk of prototype pollution. Prototype pollution occurs when an attacker can inject properties into an object prototype, which can lead to unexpected behavior and security vulnerabilities in the application. This can happen if the object being manipulated is not properly validated or sanitized.

In the provided code, the line nestedConfigObj[key] = "warn"; could potentially allow an attacker to modify the prototype of nestedConfigObj if key contains special properties like __proto__ or constructor.

To fix this issue, we should ensure that the key being assigned to is a property of the object itself and not an inherited property. This can be done using the hasOwnProperty method.

Here's the code suggestion to fix the issue:

Suggested change
nestedConfigObj[key] = "warn";
if (nestedConfigObj.hasOwnProperty(key) && nestedConfigObj[key] === "error") {

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Require statement not part of import statement.

The issue is that ESLint has flagged the use of the require statement, which is common in CommonJS modules, instead of the ES6 import statement. This can lead to inconsistencies in module imports, especially if the rest of the codebase is using ES6 modules.

To fix this, you should replace the require statement with an import statement. Here is the code suggestion to fix the issue:

Suggested change
const globals = require("globals");
import globals from "globals";

This comment was generated by an experimental AI tool.


case "eth_getBlockByNumber":
case "eth_getBlockByNumber": {

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: You have a misspelled word: eth on String

The error reported by ESLint is a false positive in this context. The linter mistakenly identifies "eth" in the string "eth_getBlockByNumber" as a misspelled word. However, "eth" is a common prefix used in Ethereum-related methods and is correctly used here.

To resolve this issue, you can add an inline ESLint comment to disable the specific rule for that line. This will prevent the linter from flagging the string as a misspelled word.

            // eslint-disable-next-line spellcheck/spell-checker
            case "eth_getBlockByNumber": {

This comment was generated by an experimental AI tool.

configObj[key] = "warn";
} else if (typeof configObj[key] === "object") {
const nestedConfigObj = configObj[key];
for (const key in nestedConfigObj) {

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: ES2015 block-scoped variables are forbidden.

The issue described by the ESLint linter is that the key variable is being declared twice within the same block scope of the changeErrorToWarn function. In ES2015 (ES6), let and const declarations are block-scoped, which means that redeclaring const key within the nested for loop is not allowed and can lead to errors.

To fix this, you can change the name of the key variable in the nested loop to avoid redeclaration. Here's the code suggestion to fix the issue:

Suggested change
for (const key in nestedConfigObj) {
for (const nestedKey in nestedConfigObj) {

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");
const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: A require() style import is forbidden.

The issue here is that ESLint has flagged the use of require() for importing the typescript-eslint module. This is likely because the project is using ES6 modules, where the preferred way to import modules is using the import statement instead of require().

To fix this issue, you should replace the require() call with an import statement.

Here's the code suggestion to fix the issue:

Suggested change
const tseslint = require("typescript-eslint");
import tseslint from "typescript-eslint";

This comment was generated by an experimental AI tool.

const stylistic = require("@stylistic/eslint-plugin");
const sort = require("eslint-plugin-sort");

const MAX_LINE_LENGTH = 120;

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: ES2015 block-scoped variables are forbidden.

The issue described by the ESLint linter is that ES2015 block-scoped variables (declared using const or let) are forbidden in this context. This might be due to an ESLint configuration that enforces the use of var instead of const or let for variable declarations.

To fix this issue, you can change the const declaration to a var declaration.

Suggested change
const MAX_LINE_LENGTH = 120;
var MAX_LINE_LENGTH = 120;

This comment was generated by an experimental AI tool.

for (const key in configObj) {
if (configObj[key] === "error") {
configObj[key] = "warn";
} else if (typeof configObj[key] === "object") {

Choose a reason for hiding this comment

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

⚠️ Codacy found a medium Security issue: Generic Object Injection Sink

The issue identified by ESLint is related to a potential security vulnerability known as a "Generic Object Injection Sink." This type of vulnerability occurs when an object is used in a way that allows for the injection of properties, which can lead to unintended behavior or security risks. Specifically, the code is checking if configObj[key] is an object using typeof, but it does not ensure that the object is a plain object. This could lead to unintended behavior if configObj[key] is an object created from a different prototype, such as an instance of a class.

To fix this issue, you should use a more robust method to check if configObj[key] is a plain object. One common approach is to use Object.prototype.toString.call to ensure that the object is indeed a plain object.

Here is the code suggestion:

Suggested change
} else if (typeof configObj[key] === "object") {
} else if (Object.prototype.toString.call(configObj[key]) === "[object Object]") {

This comment was generated by an experimental AI tool.

const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");
const stylistic = require("@stylistic/eslint-plugin");
const sort = require("eslint-plugin-sort");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: ES2015 block-scoped variables are forbidden.

The issue arises because the ESLint configuration or rules being used are forbidding the use of ES2015 block-scoped variables (such as const and let). This could be due to a specific rule that enforces the use of var for variable declarations, which is more compatible with older JavaScript environments.

To fix this issue, you should replace the const declaration with a var declaration. Here is the code suggestion to fix the issue:

Suggested change
const sort = require("eslint-plugin-sort");
var sort = require("eslint-plugin-sort");

This comment was generated by an experimental AI tool.


// Common ignored files (except the 'node_modules' folder that is ignored by default)
{
ignores: ["cache/**", "artifacts/**", "typechain-types/**"],

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: You have a misspelled word: typechain on String

The issue identified by ESLint pertains to a potential misspelling in the string "typechain-types/**". ESLint suggests that the word "typechain" might be misspelled. To address this, you should verify the correct spelling of the directory name. Assuming the intended directory name is "typechain-types", the issue might be a false positive from ESLint. However, if the actual directory name is different, you should correct it accordingly.

For the sake of this example, let's assume the correct directory name is "type-chain-types" instead of "typechain-types".

Here is the code suggestion to fix the issue:

Suggested change
ignores: ["cache/**", "artifacts/**", "typechain-types/**"],
ignores: ["cache/**", "artifacts/**", "type-chain-types/**"],

This comment was generated by an experimental AI tool.

const globals = require("globals");
const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");
const stylistic = require("@stylistic/eslint-plugin");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: A require() style import is forbidden.

The issue is that ESLint has detected the use of the require() function for importing modules, which is considered error-prone and is not recommended in modern JavaScript development. Instead, ES6 import statements should be used for module imports.

Here's the code suggestion to fix the issue by replacing the require() statement with an import statement:

Suggested change
const stylistic = require("@stylistic/eslint-plugin");
import stylistic from "@stylistic/eslint-plugin";

This comment was generated by an experimental AI tool.

const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");
const stylistic = require("@stylistic/eslint-plugin");
const sort = require("eslint-plugin-sort");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: Require statement not part of import statement.

The issue reported by ESLint is that the require statement is being used instead of an import statement. In modern JavaScript, especially when using ES6 modules, it is recommended to use import statements for better readability and consistency.

Here's the single line change to fix the issue by converting the require statement to an import statement:

Suggested change
const sort = require("eslint-plugin-sort");
import sort from "eslint-plugin-sort";

This comment was generated by an experimental AI tool.

@@ -0,0 +1,111 @@
const globals = require("globals");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: You have a misspelled word: globals on Identifier

The issue reported by ESLint is that there is a misspelled word in the identifier globals. This likely means that the module name globals is incorrect. In JavaScript, module names are case-sensitive, and globals should be corrected to match the actual module name.

Assuming the correct module name is Globals, here is the single line change to fix the issue:

Suggested change
const globals = require("globals");
const globals = require("Globals");

This comment was generated by an experimental AI tool.

const pluginJs = require("@eslint/js");
const tseslint = require("typescript-eslint");
const stylistic = require("@stylistic/eslint-plugin");
const sort = require("eslint-plugin-sort");

Choose a reason for hiding this comment

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

Codacy found a critical Error Prone issue: A require() style import is forbidden.

The issue is that ESLint has flagged the use of the require() function for importing modules as forbidden. This typically happens in environments where ES6 module syntax (import) is preferred or required for consistency, performance, or compatibility reasons.

To fix this issue, you should replace the require() statement with an import statement.

Here is the code suggestion to fix the issue:

Suggested change
const sort = require("eslint-plugin-sort");
import sort from "eslint-plugin-sort";

This comment was generated by an experimental AI tool.

@EvgeniiZaitsevCW EvgeniiZaitsevCW changed the title test: add linters to E2E tests test: a try to add linters to E2E tests Jun 12, 2024
@EvgeniiZaitsevCW
Copy link
Contributor Author

Closed as have too many issues on Codacy and I can't resolve them in the near future

@mayconamaroCW mayconamaroCW deleted the e2e-test-linter branch July 12, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant