Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

casing of ruleSeverity #2507

Closed
ChrisMBarr opened this issue Apr 5, 2017 · 6 comments
Closed

casing of ruleSeverity #2507

ChrisMBarr opened this issue Apr 5, 2017 · 6 comments

Comments

@ChrisMBarr
Copy link
Contributor

I have a set of custom rules that I'm trying to convert over to TSLint 5, but I'm having some issues getting my tests to pass due to the new ruleSeverity property. Right now my rules do not specify a severity, so they must be picking up the default severity of "ERROR" , but in my tests I'm only allowed to specify the severity names in lowercase due to this type

Here's a sample of my current test output:
2017-04-05 15_19_27-

and here's the error I get if I attempt to force it up uppercase

2017-04-05 15_25_52- appriverangularcallrequiresprefixrule tests ts - appriver tslintrules - visual

So... why does this maintain a strongly-typed list of allowed strings, but actually use a different string internally? Right now my quick fix is to just ignore the severity from the failure comparisons in my tests, I don't love it, but it works for now since none of my custom rule have a severity specified anyway.

@nchen63
Copy link
Contributor

nchen63 commented Apr 7, 2017

Is your default rule severity uppercase? Try changing it to lowercase. I'll put in a check for that

@nchen63
Copy link
Contributor

nchen63 commented Apr 7, 2017

Actually, there is a check to ensure that defaultRuleSeverity is consistent https://github.com/palantir/tslint/blob/master/src/configuration.ts#L306

Not sure how you're getting the uppercase

@ChrisMBarr
Copy link
Contributor Author

I have not defined any severity anywhere, I am migrating old rules from the TSlint 4 syntax to 5, and I'm not specifying severity anywhere, not even in a config file. Note that the red & uppercase text in the screenshot above is the "actual".

Looking into it a bit, it seems to come from this: https://github.com/palantir/tslint/blob/master/src/language/rule/rule.ts#L290

If I remove the .toUpperCase() in my local copy, it works as expected. It seems to me like this shouldn't be there.

@nchen63
Copy link
Contributor

nchen63 commented Apr 7, 2017

Yeah, this isn't ideal to have the JSON output severity as uppercase, but they are different types. Can you modify IExpectedFailure to use IRuleFailureJson instead?

@ChrisMBarr
Copy link
Contributor Author

I suppose I can just uppercase the severity in my own tests right before the comparison... just seems like a strange workaround.

@olore
Copy link
Contributor

olore commented Apr 21, 2017

I think this was my bad. @ChrisMBarr Can you see if #2622 fixes it for 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

No branches or pull requests

3 participants