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.

Sign up for GitHub

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

Jump to bottom

fix(eslint-plugin): Cast a wider net in api-no-slashes.js #8649

Merged
merged 4 commits into from
Oct 14, 2020
Merged

fix(eslint-plugin): Cast a wider net in api-no-slashes.js #8649

merged 4 commits into from
Oct 14, 2020

Conversation

christopherthielen
Copy link
Contributor

@christopherthielen christopherthielen commented Oct 12, 2020 •
edited
Loading

Simplified the detection logic for slashes.

Now detects slashes in the following new cases:

some arg is template string or string concatenation with a slash

var prefix = 'proxies/foo/v1';
API.one(`${prefix}/endpoint`);
API.one(prefix + "/endpoint"`);

some arg is reference to a template string or concatenated string with a slash

var prefix = 'proxies/foo/v1';
var url = prefix + "/endpoint";
var url = `${prefix}/endpoint`;
API.one(url);

@christopherthielen christopherthielen marked this pull request as draft October 12, 2020 23:57
@christopherthielen

This comment has been minimized.

Sign in to view
@christopherthielen christopherthielen marked this pull request as ready for review October 13, 2020 01:14
christopherthielen
christopherthielen commented Oct 13, 2020
View reviewed changes
packages/eslint-plugin/rules/api-no-slashes.js
const variableArgs = args.filter((argument) => {
if (argument.type !== 'Identifier') {
// Get the source code (think .toString()) of the AST node and find a slash
// This isn't 100% accurate, but it's good enough.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  // This isn't 100% accurate, but it's good enough.

@christopherthielen christopherthielen added the ready to merge Reviewed and ready for merge label Oct 14, 2020
@mergify mergify bot merged commit cf5ab83 into spinnaker:master Oct 14, 2020
@christopherthielen christopherthielen deleted the api-no-slashes-wider-net branch October 21, 2020 20:49
christopherthielen added a commit to christopherthielen/deck that referenced this pull request Nov 10, 2020
@christopherthielen
chore(eslint-plugin): publish 1.0.9
d218c8f 
fix(pluginsdk-peerdeps): Move eslint peer deps to plugin-eslint package ([64d5708](spinnaker@64d5708))
chore(eslint-plugin): bump jest and lodash ([96b2b86](spinnaker@96b2b86))
feat(eslint-plugin): Add a new rule prefer-promise-like to help migrate off of ng.IPromise [spinnaker#8673](spinnaker#8673) ([3dc0309](spinnaker@3dc0309))
feat(eslint-plugin): Auto-fix arbitrary expressions with slashes in api-no-slashes lint rule ([28e95ee](spinnaker@28e95ee))
fix(eslint-plugin): Allow .split('/') in API.one() calls in api-no-slashes lint rule ([e547389](spinnaker@e547389))
fix(eslint-plugin): Cast a wider net in api-no-slashes.js [spinnaker#8649](spinnaker#8649) ([cf5ab83](spinnaker@cf5ab83))
feat(eslint-plugin): Write a new rule react2angular-with-error-boundary All react2angular components should be wrapped in an error boundary ([bd9858d](spinnaker@bd9858d))
refactor(eslint-plugin): move angular-rule utils to a separate directory ([873bec3](spinnaker@873bec3))
feat(eslint-plugin): Add a new rule api-no-slashes that forbids using string literals with slashes to API.one()  [spinnaker#8629](spinnaker#8629) ([75a054e](spinnaker@75a054e))
feat(eslint-plugin): Add a README.md with instructions on creating eslint rules [spinnaker#8628](spinnaker#8628) ([6cab9e6](spinnaker@6cab9e6))
@christopherthielen christopherthielen mentioned this pull request Nov 10, 2020
Merged
mergify bot pushed a commit that referenced this pull request Nov 10, 2020
@christopherthielen
chore(eslint-plugin): publish 1.0.9 (#8718)
fbec5b8 
fix(pluginsdk-peerdeps): Move eslint peer deps to plugin-eslint package ([64d5708](64d5708))
chore(eslint-plugin): bump jest and lodash ([96b2b86](96b2b86))
feat(eslint-plugin): Add a new rule prefer-promise-like to help migrate off of ng.IPromise [#8673](#8673) ([3dc0309](3dc0309))
feat(eslint-plugin): Auto-fix arbitrary expressions with slashes in api-no-slashes lint rule ([28e95ee](28e95ee))
fix(eslint-plugin): Allow .split('/') in API.one() calls in api-no-slashes lint rule ([e547389](e547389))
fix(eslint-plugin): Cast a wider net in api-no-slashes.js [#8649](#8649) ([cf5ab83](cf5ab83))
feat(eslint-plugin): Write a new rule react2angular-with-error-boundary All react2angular components should be wrapped in an error boundary ([bd9858d](bd9858d))
refactor(eslint-plugin): move angular-rule utils to a separate directory ([873bec3](873bec3))
feat(eslint-plugin): Add a new rule api-no-slashes that forbids using string literals with slashes to API.one()  [#8629](#8629) ([75a054e](75a054e))
feat(eslint-plugin): Add a README.md with instructions on creating eslint rules [#8628](#8628) ([6cab9e6](6cab9e6))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vigneshm vigneshm vigneshm approved these changes

Assignees
No one assigned
Labels
ready to merge Reviewed and ready for merge target-release/1.24
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christopherthielen @vigneshm @spinnakerbot