Skip to content

Detect erroneous multi-line strings. #4979

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

Closed
floitschG opened this issue Sep 7, 2012 · 14 comments
Closed

Detect erroneous multi-line strings. #4979

floitschG opened this issue Sep 7, 2012 · 14 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@floitschG
Copy link
Contributor

It is a (relatively) common error to forgot commas between two items of a string list:
var a = ["foo",
         "bar" // <===
         "gee"];

In this case "bar" and "gee" will be concatenated without any warning.
It would be nice if the editor offered some kind of warning for this.
In general this is difficult, but the following heuristics could potentially work:

  • wrong number of arguments to a function call.
  • in lists: if the two strings easily fit onto one line. Here, for example, the user could just as well write "bargee" if she wanted one string. Having concatenated strings is thus probably an error.
@floitschG
Copy link
Contributor Author

Also it might be a good idea to do different syntax highlighting so that these strings stick out.

@floitschG
Copy link
Contributor Author

@danrubel
Copy link

Added this to the M2 milestone.

@clayberg
Copy link

Set owner to @bwilkerson.
Removed this from the M2 milestone.
Added this to the M3 milestone.

@danrubel
Copy link

To be added after AnalysisServer is in place


Set owner to @scheglov.
Removed this from the M3 milestone.
Added this to the Later milestone.

@scheglov
Copy link
Contributor

scheglov commented Apr 9, 2013

Set owner to @bwilkerson.
Removed Area-Editor label.
Added Area-Analyzer label.

@bwilkerson
Copy link
Member

Added Analyzer-Hint label.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@floitschG floitschG added Type-Enhancement legacy-area-analyzer Use area-devexp instead. devexp-warning Issues with the analyzer's Warning codes labels Aug 4, 2014
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed triaged labels Feb 29, 2016
@srawlins
Copy link
Member

I don't think the analyzer is going to do either of the suggestions:

  1. We already warn about wrong number of arguments to a function.
  2. I don't think the analyzer is going to start looking at whether the two strings fit on one line together... does it do any positional/formatting analysis like this today?

@bwilkerson
Copy link
Member

A request for this same functionality was just added to the linter project. It appears that it is a fairly common mistake, and we ought to provide support.

@srawlins
Copy link
Member

Ah. #57318. The list literal case. I think we have to be careful in what we're fixing, to avoid generating noise warnings.

Should there just be a hint in List literals to never use "space"-concatenated String literals? I think the request in #57318 is based on the idea that trailing plus signs are much more visible than trailing (or missing) commas.

@bwilkerson
Copy link
Member

Should there just be a hint in List literals to never use "space"-concatenated String literals?

That seems reasonable to me, although I'd make it a lint rather than a hint.

@srawlins srawlins added devexp-linter Issues with the analyzer's support for the linter package and removed devexp-warning Issues with the analyzer's Warning codes labels Apr 20, 2016
@bwilkerson bwilkerson removed their assignment Aug 15, 2016
@srawlins
Copy link
Member

This is now implemented as a lint: no_adjacent_strings_in_list, and the formatter also indents such that the error is noticeable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

8 participants