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

[new rule]: Enum cannot be: "type": "Boolean", #321

Open
jianyexi opened this issue May 31, 2022 · 6 comments
Open

[new rule]: Enum cannot be: "type": "Boolean", #321

jianyexi opened this issue May 31, 2022 · 6 comments

Comments

@jianyexi
Copy link
Contributor

jianyexi commented May 31, 2022

Lint rule description

Enum can not be: "type": "boolean",

Related swagger example

Azure/autorest.python#847
https://github.com/Azure/azure-rest-api-specs/pull/14676/files

Category

SDK

Severity level

Error

Applies to

Management plane API spec, Data plane API spec

How to fix the violation

avoid this usage

What't the impact if breaking the rule

python does not support Boolean Enum

@jianyexi
Copy link
Contributor Author

jianyexi commented May 31, 2022

@lmazuel can you help confirm this rule ? as it's from our backlog for long time not sure if it's valid now

@jianyexi
Copy link
Contributor Author

@msyyc to confirm as well

@jianyexi jianyexi added the p1 label May 31, 2022
@msyyc
Copy link
Member

msyyc commented Jun 1, 2022

Hi @jianyexi, Python has solved the compatible issue in this scenario, so it is not necessary for python now. Thanks!

@lmazuel
Copy link
Member

lmazuel commented Jun 1, 2022

@jianyexi from a big picture perspective (and not just Python), this rule is necessary to make sure we genreate SDK that makes sense. Boolean being two values only (true/false), making this an eum is an anti-pattern

@jianyexi
Copy link
Contributor Author

jianyexi commented Jun 8, 2022

@jianyexi from a big picture perspective (and not just Python), this rule is necessary to make sure we genreate SDK that makes sense. Boolean being two values only (true/false), making this an eum is an anti-pattern

thanks got it, but seems we have a rule EnumInsteadOfboolean, which is kind of conflict with the current rule, so do we prefer a string type enum instead of boolean in this case ?

@lmazuel
Copy link
Member

lmazuel commented Jul 11, 2022

It's not a conflict, they do not have the same role. EnumInsteadOfBoolean is defined as:

Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined.

This is to tell service team to not use boolean at all, but replace with strings is better on the server side. Assuming a service team has already committed to boolean and it's too late, then they must NOT use an enum that wrap that boolean. Not the same abstraction level.

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

No branches or pull requests

3 participants