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

Commenting out a line of code that starts with 'member' triggers 'Unexpected /*member' error #881

Closed
ghost opened this Issue Feb 28, 2013 · 4 comments

Comments

Projects
None yet
3 participants
@ghost

ghost commented Feb 28, 2013

I had a line of code that I wound up commenting out for testing reasons, something along the lines of:

    // memberProducts.push(product);

    View.userData = {
        City: data.city,
    ...

After commenting the line out, I started receiving a ton of errors, starting with:

Unexpected /*member 'userData'
Unexpected /*member 'City'

going down my code until there were too many errors to continue.

After many hours of frustration, the following fixed the issue:

    //blah memberProducts.push(product);

It seems like JSHint isn't properly distinguishing between a commented line of code that starts with 'member' and a definition of a 'member' for linting purposes.

@valueof

This comment has been minimized.

Show comment
Hide comment
@valueof

valueof Mar 1, 2013

Member

Yeah, this happens because JSHint now supports // member ... in addition to /*member ... */.

Member

valueof commented Mar 1, 2013

Yeah, this happens because JSHint now supports // member ... in addition to /*member ... */.

@Krinkle

This comment has been minimized.

Show comment
Hide comment
@Krinkle

Krinkle Mar 5, 2013

The following code (extracted and simplified from a larger library) keeps failing as of JSHint 1.0:

/*global FOO */
/*jshint forin:false, laxbreak:true */
( function () {
  'use strict';

  FOO.fn = {
    injectCheck: function ( masterVariable, objectPathArray, injectFn ) {
      var i, len, prev, memberName, lastMember,
          curr = masterVariable;

      // This is a simpified version of a long inline
      // comment that used to be in this exact place
      // [..snip..]
      // member and keep references to the parent object, member name and member
      // [...end of snip..]
      for ( i = 0, len = objectPathArray.length; i < len; i++ ) {
        memberName = objectPathArray[i];

        prev = curr;
        curr = prev[memberName];
        lastMember = memberName;
      }

      prev[lastMember] = function () {
        injectFn();
        return curr.apply( this, arguments );
      };
    }
  };

}() );

Causing:

Unexpected /*member 'length'.
Unexpected /*member 'apply'.
Unexpected /*member 'injectCheck'.

I found this issue so reporting here instead.

I don't know what the advantage of supporting // member ... is, but unless it is smarter about false-positives (e.g. when the value is not a comma-separated list of valid identifiers, or when there are other inline comments before and after), it should imho be removed as it is doing more harm than good. Or at least provide a way to disable it.

I don't intend to rewrite all comments and have to worry about future comments not breaking before the word "member", that's just crazy and unusable.

This is currently blocking me from upgrading back to jshint 1.0, I reverted all my usage back to 0.9.

Krinkle commented Mar 5, 2013

The following code (extracted and simplified from a larger library) keeps failing as of JSHint 1.0:

/*global FOO */
/*jshint forin:false, laxbreak:true */
( function () {
  'use strict';

  FOO.fn = {
    injectCheck: function ( masterVariable, objectPathArray, injectFn ) {
      var i, len, prev, memberName, lastMember,
          curr = masterVariable;

      // This is a simpified version of a long inline
      // comment that used to be in this exact place
      // [..snip..]
      // member and keep references to the parent object, member name and member
      // [...end of snip..]
      for ( i = 0, len = objectPathArray.length; i < len; i++ ) {
        memberName = objectPathArray[i];

        prev = curr;
        curr = prev[memberName];
        lastMember = memberName;
      }

      prev[lastMember] = function () {
        injectFn();
        return curr.apply( this, arguments );
      };
    }
  };

}() );

Causing:

Unexpected /*member 'length'.
Unexpected /*member 'apply'.
Unexpected /*member 'injectCheck'.

I found this issue so reporting here instead.

I don't know what the advantage of supporting // member ... is, but unless it is smarter about false-positives (e.g. when the value is not a comma-separated list of valid identifiers, or when there are other inline comments before and after), it should imho be removed as it is doing more harm than good. Or at least provide a way to disable it.

I don't intend to rewrite all comments and have to worry about future comments not breaking before the word "member", that's just crazy and unusable.

This is currently blocking me from upgrading back to jshint 1.0, I reverted all my usage back to 0.9.

@valueof

This comment has been minimized.

Show comment
Hide comment
@valueof

valueof Mar 5, 2013

Member

Yeah I agree, will fix that.

Member

valueof commented Mar 5, 2013

Yeah I agree, will fix that.

@valueof valueof closed this in 58774d5 Mar 6, 2013

@amadeus

This comment has been minimized.

Show comment
Hide comment
@amadeus

amadeus Apr 1, 2013

I really liked the single line comment architecture, what if the API could allow for a sort of prefix namespacing?

// jshint-global alert, dog, etc

It could be simplified to even jsh- or something, but that would easily allow it to avoid false positives.

Thoughts?

To summarize:
I think to maintain backwards compatibility, what is committed for v1.1 should stay, just the possibility of a prefix namespace to still allow for single line comments. And yes, I do realize it makes the API a bit more confusing, I just really liked the single line comments for // global

amadeus commented Apr 1, 2013

I really liked the single line comment architecture, what if the API could allow for a sort of prefix namespacing?

// jshint-global alert, dog, etc

It could be simplified to even jsh- or something, but that would easily allow it to avoid false positives.

Thoughts?

To summarize:
I think to maintain backwards compatibility, what is committed for v1.1 should stay, just the possibility of a prefix namespace to still allow for single line comments. And yes, I do realize it makes the API a bit more confusing, I just really liked the single line comments for // global

jugglinmike added a commit to jugglinmike/jshint that referenced this issue Oct 21, 2014

Remove support for single-line special comments (except for //jshint)
Single-line special comments make more harm than good. Closes jshintGH-881.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment