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

Indent rule: unable to enforce indentation of switch cases [$15 awarded] #1797

Closed
lo1tuma opened this issue Feb 8, 2015 · 11 comments

Comments

Projects
None yet
@lo1tuma
Copy link
Member

commented Feb 8, 2015

The following code results in having no errors:

eslint --reset --no-eslintrc --rule "{indent: 2}" test.js

switch (a) {
case '1':
b();
break;
default:
c();
break;
}

If I enable the indentSwitchCase option I get the following errors:
eslint --reset --no-eslintrc --rule "{indent: [2, 4, {indentSwitchCase: true}]}" test.js

test.js
  3:4  error  Expected indentation of 4 characters  indent
  6:4  error  Expected indentation of 4 characters  indent

I would expect the following errors:

test.js
  2:4  error  Expected indentation of 4 characters  indent
  3:8  error  Expected indentation of 8 characters  indent
  4:8  error  Expected indentation of 8 characters  indent
  5:4  error  Expected indentation of 4 characters  indent
  6:8  error  Expected indentation of 8 characters  indent
  7:8  error  Expected indentation of 8 characters  indent

It seems like the indentSwitchCase option enables the indentation check only of the body of a SwitchCase statement. The Indentation of the whole SwitchCase statement is recognized by the rule itself and only validated to be consistet within an single switch statement. There is no option to confiugre a consistent identation for SwitchCase statements of a whole file.

The other strange behavior with this option is, when I explecitly set the value to false it doesn’t forbid the identation for a SwitchCase body, it just ignores those lines.

The $15 bounty on this issue has been claimed at Bountysource.

@chpio

This comment has been minimized.

Copy link

commented Mar 15, 2015

i have a strange "indent": [2, "tab", {indentSwitchCase: true}]behavior too.

my code:

switch(test) {
    case '2':
        test(24)
        //falls through

    case '3':
        test(42)
        break
}

the error for "indent": [2, "tab", {indentSwitchCase: true}]:

  23:2  error  Expected indentation of 2 characters  indent
  24:3  error  Expected indentation of 3 characters  indent
  25:3  error  Expected indentation of 3 characters  indent

so i have to change the code to:

switch(test) {
    case '2':
        test(24)
        //falls through

        case '3':
            test(42)
            break
}
@doberkofler

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2015

@t128 I'm also using the indent rule in an physical tab environment and cannot reproduce your problem. The following code example shown no errors in eslint 0.16.2 when using the tab character to indent by one or two levels.

/*eslint indent: [2, "tab", {indentSwitchCase: true}]*/

var test = "1";
switch(test) {
    case "2": // indented by 1 tab character
        test(24); // indented by 2 tab characters
        // falls through
    case "3":
        test(42);
        break;
}

@ilyavolodin ilyavolodin changed the title Indent rule: unable to enforce indentation of switch cases Indent rule: unable to enforce indentation of switch cases [$15] Apr 22, 2015

@ilyavolodin ilyavolodin added the bounty label Apr 22, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Apr 26, 2015

@lo1tuma I started working on it.
Based on your example on top, you expected line 4 to have an indention of 8 characters.
Further looking into the code I found out that it considers break statement to along the same column as case statement.
This is weird for me also. As I also expect the indentation as you expected for break statement.
Do you think the rule should be changed to accommodate this scenario also.

Any input @nzakas ?
FYI, I am just talking about the switch case here.

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 26, 2015

Yeah, I'd expect break to be indented by default.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 12, 2015

I am going to re-write the whole ident rule and also fix this issue.
I already have it working.

gyandeeps added a commit that referenced this issue Jul 12, 2015

gyandeeps added a commit that referenced this issue Jul 13, 2015

gyandeeps added a commit that referenced this issue Jul 13, 2015

gyandeeps added a commit that referenced this issue Jul 13, 2015

@gyandeeps gyandeeps closed this in 5ec47de Jul 14, 2015

nzakas added a commit that referenced this issue Jul 14, 2015

Merge pull request #2978 from eslint/IndentRuleFix
Fix: Indent rule (fixes #1797, fixes #1799, fixes #2248, fixes #2343,…
@joeybaker

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2015

Thank you @gyandeeps!

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 14, 2015

u bet 👍

@nzakas nzakas changed the title Indent rule: unable to enforce indentation of switch cases [$15] Indent rule: unable to enforce indentation of switch cases [$15 awarded] Sep 17, 2015

@CodisRedding

This comment has been minimized.

Copy link

commented Dec 11, 2015

This is still an issue with the latest version. screenshot http://bad-request.fourq.io/0y102l321D3p

@mgenware

This comment has been minimized.

Copy link

commented Jan 2, 2016

@FourQ try this:

"indent": [2, 2, {"SwitchCase": 1}]

More info

@awitherow

This comment has been minimized.

Copy link

commented May 10, 2016

@mgenware that works for me as well, thanks for the hookup 👍

aaronjudd pushed a commit to reactioncommerce/reaction that referenced this issue Aug 5, 2016

Force eslint to recognize that switch/case should be indented (#1250)
* Force eslint to recognize that swtich/case should be indented
eslint/eslint#1797

* remove eslint-plugin-meteor

- we need to tune this, removing until we have time

* enable eslint-2 codeclimate

- remove meteor-eslint plugin, failing to load with eslint-2

* updated switch indentation

- test eslint switch, case indent

kieckhafer added a commit to reactioncommerce/reaction that referenced this issue Aug 18, 2016

Release v0.15.0 (#1308)
* create launchdock-connect plugin (#1241)

* rebuild launchdock-connect as a plugin module

* build more flexible admin email verification method

* add Stripe dependency for launchdock-connect

* fix linting issues

* Adjust inventory on shipment (#1240)

* Trap order-completed change

* Add missing import

* Add cartItemId field to order so that we can modify inventory

* Add new "inventory/sold" method

* Explicitly set from and to statuses

* Handle order insert hook to move ordered inventory to "sold" status

* Handle moving inventory from "reserved" to "sold" status

* Fix email import

* Fix tests

* Not so much logging

* Call status change method with explicit values

* Add test for moving inventory from "reserved" to "sold"

* Add test for moving inventory from "sold" to "shipped"

* Force eslint to recognize that switch/case should be indented (#1250)

* Force eslint to recognize that swtich/case should be indented
eslint/eslint#1797

* remove eslint-plugin-meteor

- we need to tune this, removing until we have time

* enable eslint-2 codeclimate

- remove meteor-eslint plugin, failing to load with eslint-2

* updated switch indentation

- test eslint switch, case indent

* Add route hooks API (#1253)

* build initial implementation of route hooks

* extend Router with Hooks and register all hooks

* more route hooks tweaks

* router namespace cleanup

* removed unused values from registry entry (linter)

* Refactor inventory (#1251)

* As noted in comments from previous PR, just some reorg kept separate from the functionality PR

* Correct matching

* Fix for decrement cart function  (#1273)

* Add failing test

* Fix for update Mongo command

* Add failing test for "decrease below zero"

* If removeQuantity is more than quantity remove the entire line

* updated linting error

* linting error fix

* listing issues

* more cleanup and linting tweaks

* fix eslint config for object key quotes

* fix tests after error message update

* revert quote-props linter change

* revert lint changes

* Simplify logic per CR comment

* import moment-timezone (#1261)

- fixes error .names undefined
- for loading tz in i18n settings

* Fix inventory tests (#1254)

* Reduce size of the cart so that orders don't take so long to process.

* Add a wait before pulling record

* Add a wait before pulling record

* Wait after calling Meteor method

* Longer timeout

* Longer timeout

* Temporarily bypass failiing inventory test (#1280)

* fix for #1072 (#1247)

fix for #1072

* Use forked version of authorize.net that doesn't have vulnerability (#1252)

* Update circle node (#1281)

* update circle node to v4.4.7

* remove unused packages

- remove eslint-plugin-meteor
- remove bunyan-loggly (insecure dependency)
- currently not in use

* updated package.json (#1286)

- updates jquery-i18next
- updates i18next-browser-languagedetector

* Create email job queue and Reaction.Email namespace (#1282)

* build job queue for email and refactor related methods

* put placeholder content in coreDefault email template

* add rate limiting for accounts and notification methods

* put order completed template at correct path

* update old jquery-i18next

* remove log

* remove unused var

* remove unnecessary filler in placeholder template

* log warning when email template isn’t found

* return job object from Reaction.Email.send()

* fix missing import

* fix comment for email method

* remove debugging logger

* updated to remove name from email

should be enough info

* logout and hasPermission updates (#1290)

* updated hasPermissions, router behaviour

- resolves #1122
- refactors client hasPermissions to wait for a userId if one isn’t
immediately found
- intentionally not redirecting to home page (not sure if that’s the
best behavior, better to have login?)
- adds subscription manager to a few more collections

* fix typo

* import lodash

might work better if _.find exists

* check for existing route table

* Fix PayPal PayFlow discounts and refunds (#1275)

* allow discounts and refunds with PayPal PayFlow

- Fix discounts so they are working
- Allow 100% discounts
- Fix refunds so they are working

* wrap PayPal PayFlow in wrapper for easier testing

* PayPal PayFlow refund test

* removed temporary file

* updated cc error message

* update cardNumber schemas

cardNumber schemas were not allowing valid credit cards, such as Amex
(15 digits), some Visa (13 digits), and foreign Maestro (12 - 19
digits). This updates to allow these lengths to be input.

* Remove bash script from postinstall to fix Windows installs (#1299)

* don’t run bash scripts in postinstall because Windows

* add fallback fonts back to public dir

* release 0.15.0

- updated package in preparation for new release

* Allow any user who has the createProduct permission to also delete products (#1263)

* - ignoring my custom entrypoint (for running prod image with src)

* - allow users with createProduct permission to delete products (for marketplace withmultiple sellers)

* - removing .gitignore files that is not welcome in repo

* Fix Braintree discounts and refunds (#1265)

* add Braintree Payment error to en.json

* enable discounts for Braintree payments

Braintree was not capturing discounts, instead using the amount in the
original authorization for the capturing. This update changes the
amount to the paymentMethod.amount, which includes discounts.

* removed else statement after an if containing a return

Since the return inside the IF statement will effectively kill the
process if it’s hit, there is no need for the else.

* added testing for Braintree refunds

This code still needs some love, just wanted to get it up for others to
take a look at.

* remove no longer needed commented code

* remove no longer needed commented code

* added field to expected response for testing

* moved braintree/refund/list methods into BraintreeApi wrapper

Creating a wrapper for Braintree in order to more easily perform testing

* move braintree methods into new file

* reconfigure all braintree payment code to no longer use ValidateMethod

* fixed lint issues

* removed code used to skip over 24 hour braintree delay (for testing)

* Rename braintreeapi.js to braintreeApi.js

* display absolute number of adjustedTotal

due to various instances of rounding numbers for display purposes, the
adjustedTotal would sometimes display -0.00, as the adjusted total was
technically -.00000000000000000000001, even though we display 0.00 when
we round it. This update just shows the absolute number, as this is
simple a display number and does not have any affect on what is being
sent to and from the payment provider.

* updated schema to match supported payment methods

The current Schema had a 16 number minimum for credit cards, however
braintree supports cards which have number lengths ranging from 12-19

* min -> max

* removed comments

* test testing

* update braintree test

* update exports of braintreeApi functions

* fixed callback error when action has no callback

* Updated error message to make more sense to a human user

* linter fixes

* updated 'Logger.info' to Logger.debug

* removed unused test

* removed commented callback

* don't log full order details on transaction error

* fix refunds and discounts for authorize.net (#1279)

- Fix discounts so that they are correctly sent to authorize.net

- Allow 100% discounts by adding the “voidTransaction” function and
voiding transactions

- Update error messaging for Refunds, as we do not (yet) allow them
from Authorize.net

* Fix Stripe refunds and Double Discounts (#1304)

* added comment to explain voiding vs applying discount

* add ___ for correctly calculating Stripe adjusted totals

Stripe doesn’t “do” discounts, instead they take the discount sent to
them, and apply it as a refund. This was causing any discounts to show
up twice - once as a discount, once as a refund - in our adjusted
total. This update fixes that by re-adding the discount price into the
adjusted total when the payment provider is Stripe.

* also 100% discounts on Stripe

* humanizing an error code

* removed unintentional text

* linting fixes

* removed unused commented code

* removed unused commented code

* Taxes (#1289)

* initial commit tax-rebase plugin

Initial commit for issue #972

* split providers to plugins/included

* Griddle updates

- fork MeteorGriddle into core/ui-grid
- move fetchTI
- add i18n taxSettings

* initial grid editing

* add taxSettings labels

* initial custom add / edit tax rates

- initial working forms, edit toggling.. bit rough..

* rename tax-base to taxes

* updated custom grid

 - adds row selection

* add custom rate form reset

* updated custom settings

* add cart hooks to trigger taxes/calculate

* update hooks, taxes/deleteRate

- also adds taxes/deleteRate

* migrate schemas to plugins

- also set taxCloud jobs to not run by default

* add custom tax rates calculation

- add taxes, tax to cart schema
- taxes not published to client
- updates global cartTotal helpers

* check for shipping

* migrate settings to plugins

* add tax hooks to plugins

 -abort idea on “provides” as a package property

* Avalara Tax Lookup

 - adds taxes/setRate method

* comment unused fields

- comment out first implementation unused fields

These are fields that will be used future enhancements to taxes.  They
are commented out for now.

* logger cleanup

- move info to debug

* updated product data with default taxable items

- add taxCode placeholder to schema

* taxCloud tax rate calculations

* disable taxjar, check for packages

- disable while module is not functional
- WIP fetch rates and api configuration

* custom rate ui cleanup

* taxes method test

- a weak attempt at a test but gets the ball rolling and actually fixes
a todo

* updated test, versions

- add done to test
- updated meteor package versions

* remove unused template registry entries

- final cleanup, ready for PR to development
- pending tests
- pending docs

* avoid running country-data unless address exists

* add tax rate delete

- adds delete and confirm to edit form

* linted griddle.js

* remove unused import

* fix display of taxes in order workflow

perhaps this should be updated to “tax”, instead of taxes in the
invoice object.

* line item calculations

- added for custom rates
- added for avalara

rates are calculated by individual line items but on a general rate for
the cart.

* update case linted

* updated variant methods

- update products/updateProductField to handle boolean
- kludge update of childVariants when parentVariant is updated
- remove default value from form
- small cleanups

hisabimbola added a commit to commercetools/eslint-config that referenced this issue Nov 9, 2016

fix(rules): switch statements
currently eslint is reporting incorrect indentation on switch statements, this commit fixes that.

More info [here](eslint/eslint#1797)

@hisabimbola hisabimbola referenced this issue Nov 9, 2016

Merged

fix(rules): switch statements #16

0 of 2 tasks complete

hisabimbola added a commit to commercetools/eslint-config that referenced this issue Nov 10, 2016

fix(rules): switch statements (#16)
currently eslint is reporting incorrect indentation on switch statements, this commit fixes that.

More info [here](eslint/eslint#1797)
@xyzdata

This comment has been minimized.

Copy link

commented Jun 29, 2017

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.