Skip to content

Codelyzer doesn't support export default #778

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

Closed
wKoza opened this issue Feb 25, 2019 · 5 comments · Fixed by #788
Closed

Codelyzer doesn't support export default #778

wKoza opened this issue Feb 25, 2019 · 5 comments · Fixed by #788
Labels

Comments

@wKoza
Copy link
Collaborator

wKoza commented Feb 25, 2019

I reedit my comment form the PR #754.

If we create a simple class like that:

export default class { ... }

and we launch ng lint, we have a big big crash [here] (https://github.com/mgechev/codelyzer/blob/master/src/propertyDecoratorBase.ts#L41) and many other places !

@wKoza wKoza added the bug label Feb 25, 2019
@mgechev
Copy link
Owner

mgechev commented Feb 25, 2019

That's probably in the Angular router where we collect the metadata?

@wKoza
Copy link
Collaborator Author

wKoza commented Feb 25, 2019

Each call to visitClassDeclaration is often following by node.name!.text, here or there and without name ... it's the crash
I'll do a complete check tomorrow.

@rafaelss95
Copy link
Collaborator

rafaelss95 commented Feb 25, 2019

I agree that instead of using the non-null assertion operator, we should check all these properties correctly!

However, I disagree on the point of returning class names (not just class names) in failure messages because I don't see much value in returning them since the lint already returns the exact line where failure occurred accompanied by a (descriptive) message.

I really prefer to return generic/direct messages without polluting the output with class/variable names, as TSLint does, for example.

One example is use-pipe-transform-interface:

Current message: The CLASSNAME class has the Pipe decorator, so it should implement the PipeTransform interface
Direct message: (or something like this): Classes decorated with @Pipe decorator should implement PipeTransform interface.

I hope you got the idea.

@wKoza
Copy link
Collaborator Author

wKoza commented Feb 26, 2019

I didn't go much more deeper into the analyse. But, I can say that a class with an Angular decorator and no name should be throw an error.
@rafaelss95 , your idea is interesting. A generic message allows to make statistic easily.

@mgechev
Copy link
Owner

mgechev commented Feb 26, 2019

@rafaelss95 yes, this makes sense.

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