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

Rule Change: no-console could be fixable #17493

Closed
1 task done
edi9999 opened this issue Aug 23, 2023 · 8 comments · Fixed by #17680
Closed
1 task done

Rule Change: no-console could be fixable #17493

edi9999 opened this issue Aug 23, 2023 · 8 comments · Fixed by #17680
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@edi9999
Copy link

edi9999 commented Aug 23, 2023

What rule do you want to change?

no-console

What change to do you want to make?

Implement autofix

How do you think the change should be implemented?

A new option

Example code

I debug a lot with console.log, and have to remove them all afterwards.

Some console.log I write are multiline, like this :

console.log(
	JSON.stringify({
		msg: "inspect-sectObj",
		inspect: util.inspect(sectObj, { showHidden: true, depth: 2 }),
	})
);

What does the rule currently do for this code?

It will simply report the error

What will the rule do after it's changed?

It could fix the error, removing the console.log alltogether

Participation

  • I am willing to submit a pull request to implement this change.

Additional comments

I know this has not been implemented on purpose, probably people don't want all their console.logs dropped by eslint by default, I would also like to be able to make this rule fix only on certain conditions.

@edi9999 edi9999 added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules labels Aug 23, 2023
@edi9999
Copy link
Author

edi9999 commented Aug 23, 2023

For the moment I will continue removing my console.log manually, it is just (a bit) annoying when you do it twice a day for 5 minutes and you know you will do so in the future 5-10 years)

@mdjermanovic
Copy link
Member

Hi @edi9999, thanks for the proposal!

We can't implement this as autofix, because autofixes should never change the runtime behavior of code. This is the policy for core rules and general advice for plugins (though, you can find an equivalent but autofixable no-console rule in eslint-plugin-autofix).

We could only consider implementing this in the core no-console rule as suggestions, if that seems useful to you?

@edi9999
Copy link
Author

edi9999 commented Aug 23, 2023

Interesting, how can I access the suggestions ?

Is there a way to access them from the command line ? If yes, I would be interested in first having an example output in the command line, then I could write the code for the suggestion of the console.log rule.

@mdjermanovic
Copy link
Member

Suggestions can be applied through editors that support this feature.

For example, in VS Code:

image

(Remove the `\`... and Replace the `\`... are two suggestions; when user selects a suggestion, editor applies the fix).

There's no built-in support for applying suggestions from the command line.

@Dimava
Copy link

Dimava commented Aug 27, 2023

Removing console with autofix should be a standalone plugin
And it already exists as https://www.npmjs.com/package/eslint-plugin-fulldo-remove-console
(I would prefer an on-github alternative with more options, this one can only remove console.logs with no rule config)

@edi9999
Copy link
Author

edi9999 commented Aug 28, 2023

So you mean you wouldn't be open to add the "suggestion" to console.logs ?

I'm using neovim as my editor so I'm not sure whether I can use suggestions at all, that's why I asked whether I could do something from the command line.

@mdjermanovic
Copy link
Member

The ESLint team would be open to evaluating adding suggestions to the no-console rule.

I'm using neovim as my editor so I'm not sure whether I can use suggestions at all, that's why I asked whether I could do something from the command line.

Not sure either, but you could try with a code example where suggestions are expected to appear:

/* eslint no-useless-escape: "error" */

"\a";

The expected suggestions are as in this demo.

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Sep 20, 2023
@nzakas
Copy link
Member

nzakas commented Sep 20, 2023

Marking as accepted.

@mdjermanovic mdjermanovic linked a pull request Sep 20, 2023 that will close this issue
1 task
Rec0iL99 added a commit to Rec0iL99/eslint that referenced this issue Oct 25, 2023
Rec0iL99 added a commit to Rec0iL99/eslint that referenced this issue Oct 27, 2023
Rec0iL99 added a commit to Rec0iL99/eslint that referenced this issue Oct 27, 2023
Rec0iL99 added a commit to Rec0iL99/eslint that referenced this issue Oct 27, 2023
Rec0iL99 added a commit to Rec0iL99/eslint that referenced this issue Nov 4, 2023
Rec0iL99 added a commit to Rec0iL99/eslint that referenced this issue Nov 7, 2023
mdjermanovic pushed a commit that referenced this issue Nov 17, 2023
* feat: Add suggestions to no-console

Fixes #17493

* Fix method of when to show safe suggestions and fix tests.

* improved formatting of tests with suggestions by breaking them into multiple lines

* Updated function name to canProvideSuggestions. Added better description for the function.

* Fixed code that fails when AST is not deep enough.

* Added suggestions:null for test cases that will not provide a suggestion.

* test to make sure console statement with semicolon is removed via suggestion

* dont provide suggestions if removing console.log() will lead to ASI breaking

* missing period

* renamed regexps variable names for better understanding

* updated passing in expressionstatement node instead of memberexpression node to maybeAsiHazard

* ++ or -- in the token before is not always safe.
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators May 16, 2024
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
Archived in project
6 participants
@nzakas @edi9999 @Dimava @mdjermanovic @Rec0iL99 and others