Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Add disallowMultipleSpacesInRegularExpressionLiterals lint rule #121

Conversation

ikeryo1182
Copy link
Contributor

Add no-regex-spaces lint rule on #94

enter(path: Path): AnyNode {
const {context, node} = path;

const multipleSpacesPattern = /( {2,})(?: [+*{?]|[^+*{?]|$)/gu;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you hoist this regex to the top level of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good. I fixed.

@sebmck
Copy link
Contributor

sebmck commented Mar 3, 2020

This is missing normal regex literals. Is there any reason you didn't add a handler for RegExpLiteral?

@sebmck sebmck mentioned this pull request Mar 3, 2020
27 tasks
@ikeryo1182
Copy link
Contributor Author

This is missing normal regex literals. Is there any reason you didn't add a handler for RegExpLiteral?

Sorry, it is the lack of consideration
I added.

@ikeryo1182 ikeryo1182 requested a review from sebmck March 4, 2020 01:37
Comment on lines 25 to 27
(node.type === 'ExpressionStatement' &&
node.expression.type === 'RegExpLiteral' &&
multipleSpacesPattern.test(node.expression.pattern))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be:

Suggested change
(node.type === 'ExpressionStatement' &&
node.expression.type === 'RegExpLiteral' &&
multipleSpacesPattern.test(node.expression.pattern))
(node.type === 'RegExpLiteral' &&
multipleSpacesPattern.test(node.pattern))

It's overly specific because ExpressionStatement is only used as the wrapper statement for an expression. A regexp should never appear on a line by itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see and accept suggestion, also fix conflict as soon as possible.

ikeryo1182 and others added 2 commits March 4, 2020 14:46
…pacesInRegularExpressionLiterals.ts

Co-Authored-By: Sebastian <853712+sebmck@users.noreply.github.com>
@ikeryo1182
Copy link
Contributor Author

ikeryo1182 commented Mar 4, 2020

@sebmck
Hi.
Do you know why this below error occured in CircleCI ?


  ✖ Timed out after waiting 3000ms for Bridge.handshake

  1. Timeout._onTimeout (C:\Users\RUNNER~1\AppData\Local\Temp\rome-dev\index.js:4987:12)
    4985 │             listener.unsubscribe();
    4986 │             reject(
  > 4987 │             new Error('Timed out after waiting ' + timeout + 'ms for ' + this.name));
         │             ^ 
    4988 │           }, timeout);
    4989 │         }

  2. listOnTimeout (internal/timers.js:549:16)
  3. process.processTimers (internal/timers.js:492:6)

This diagnostic was derived from an internal Rome error. The problem likely isn't with your code. Please report this if necessary

It seems that all tests passed in local.

@sebmck
Copy link
Contributor

sebmck commented Mar 4, 2020

Might be a transient error, nothing here that should cause it. Let me rerun CI and I'll see if it passes.

@sebmck
Copy link
Contributor

sebmck commented Mar 4, 2020

Looks to be passing now, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants