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

Detect import.meta as module in .js files #4399

Merged
merged 2 commits into from
Jul 25, 2022
Merged

Detect import.meta as module in .js files #4399

merged 2 commits into from
Jul 25, 2022

Conversation

lucaseverett
Copy link
Contributor

@lucaseverett lucaseverett commented Jul 7, 2022

Fixes #4331 and #3639

When using import.meta in a .js file, the linter returns an error, "Cannot use 'import.meta' outside a module", unless the source type is detected as a module by the use of an import/export. This patch parses the code as a script if a .js file contains import.meta and it hasn't already been detected as a module by the use of an import/export. If the specific import.meta error is found during the catch block, then the source type is set as a module.

This patch does result in the code being parsed again, but that happens only in the specific situation that currently causes an error. Considering that the use of import.meta within a .js file is currently preventing addons from being able to pass the linter, I believe this is an acceptable workaround.

A unit test has also been added to test the patch.

@codecov
Copy link

codecov bot commented Jul 7, 2022

Codecov Report

Merging #4399 (b6effb9) into master (00b585e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4399   +/-   ##
=======================================
  Coverage   98.71%   98.71%           
=======================================
  Files          54       54           
  Lines        2811     2812    +1     
  Branches      840      840           
=======================================
+ Hits         2775     2776    +1     
  Misses         36       36           
Impacted Files Coverage Δ
src/scanners/javascript.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00b585e...b6effb9. Read the comment docs.

@willdurand willdurand self-requested a review July 11, 2022 12:00
Copy link
Member

@willdurand willdurand left a comment

Choose a reason for hiding this comment

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

Hey, sorry for the delay. That looks promising, thanks!

tests/unit/scanners/test.javascript.js Outdated Show resolved Hide resolved
src/scanners/javascript.js Outdated Show resolved Hide resolved
@lucaseverett
Copy link
Contributor Author

The requested changes have been made.

Copy link
Member

@willdurand willdurand left a comment

Choose a reason for hiding this comment

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

Thanks for updating the patch. Could you please re-add the test case you had initially? We still need to cover the change for import.meta.

@lucaseverett
Copy link
Contributor Author

The unit test has been re-added.

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.

Linter objects to use of import.meta while extension works just fine
3 participants