Support for blank line after block comment when fixing a file

[moshest]

The problem is that files without extra line space fix incorrectly and duplicate the licence comment:

The original file:

/**
 * Copyright 2017 Moshe Simantov
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
console.error("Don't run `mocha` directly. Use `make test`.");
process.exit(0);

The fix:

/**
 * Copyright 2017 Moshe Simantov
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

/**
 * Copyright 2017 Moshe Simantov
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
console.error("Don't run `mocha` directly. Use `make test`.");
process.exit(0);

Please see issue #2 for my configuration.

My template file is as follow (notice the blank line on the end):

/**
 * Copyright <%= YEAR %> Moshe Simantov
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

[moshest]

I think that the best solution for this bug is not to fix anything and let the developer fix it manually.

In case there is an old version of the comment, let's say it was APACHE-2 and the developer decide to change it to MIT. The --fix shouldn't add the comment above it, it should do nothing and let the developer to remove the old comment first and only then it could be fixed.

[nickdeis]

Hey moshest,
Thanks for filing this issue. This looks a lot like issue #1. If you are running less than 0.2.3, then you may run into this issue.
What does npm ls eslint-plugin-notice return?
Thanks,
Nick

[moshest]

Nope.

eslint-plugin-notice@0.2.3

[nickdeis]

Hey moshest,
I think this is actually a defect in this case. The other use case you describe should be handled by having a more complicated regular expression. It is a very interesting use case though, so I'm open to ideas on how to implement it.
Best,
Nick

[nickdeis]

Hey @moshest,
I've created a unit test for the issue you described, but it's not applying --fix since it the code it's linting has a valid header. Am I missing something?

[moshest]

Yes. My configuration was that the original file is not a valid header. I added a space line after the comment.

This is a valid header:

/**
 * Copyright 2017 Moshe Simantov
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

console.error("Don't run `mocha` directly. Use `make test`.");
process.exit(0);

This is not:

/**
 * Copyright 2017 Moshe Simantov
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
console.error("Don't run `mocha` directly. Use `make test`.");
process.exit(0);

[nickdeis]

Ah now I see. Thank you for explaining that.
So perhaps what you are talking about is some sort of "header detection". Where it might not be the header you want, so eslint should throw an error instead of fixing it.
I might be able to make that an option since I already have to detect for headers anyways in the code.
Maybe call it onNonMatchingHeader and have the options of fix,error, or warn.
That's actually very useful! At least I could see me wanting to use that at some point.
I'll probably implement this myself if that's okay.
Thanks!
Nick

[nickdeis]

Hey @moshest,
This is fixed as of 0.3.3.
Unfortunately, I couldn't added support for error/warn. I can only report at the level assign to the rule. To make up for that I added "replace", which I think a lot of people will find useful.
The docs have been updated as well.

onNonMatchingHeader

• prepend: Prepends the fix template, if it exists, leaving the former header comment intact.
• replace: Replaces the former header comment with the fix template if it exists
• report: Does not apply fix, simply reports it based on the level assigned to the rule ("error" or "warn")

[nickdeis]

Feel free to tell me how it goes, quite interested!

[moshest]

Hi,

Sorry to tell you by I got the same behaviour when I'm applying --fix.

Version: 0.3.3
Test file: same as above

Config:

    'notice/notice': ['error',
      {
        mustMatch: copyrightMatch,
        template: copyrightTemplate,
        onNonMatchingHeader: 'report',
      },
    ],

[nickdeis]

Hey @moshest,
Could you take a look at test case 4? I'm having trouble recreating this.
If you have any questions, feel free to ask.
Best,
Nick

[moshest]

Regarding the onNonMatchingHeader bug. I have looked at the code..

Looks like that if onNonMatchingHeader === "report" you report anything without checking if !String(text).match(mustMatch). You should first check for match and only then report.

[nickdeis]

Yea I suppose that does make sense 😂. Why are my unit tests passing then? Either way, I'll fix that.
Thanks for reviewing my code!

[nickdeis]

Alright, made that change. I'm still curious to see why my unit tests passed

[nickdeis]

Closing issue. Please reopen if you run into any problems.