[Typescript] Angular dependency injection and readonly

Hi guys,

We’re using TypeScript Rules in an Angular project and we have some problems withe the rule:

Private properties that are only assigned in the constructor or at declaration should be “readonly”

In fact, we use dependency injection in an angular component, as it’s done in angular.io documentation (https://angular.io/tutorial/toh-pt4#inject-the-heroservice) and in the angular styleguide (https://angular.io/guide/styleguide):

constructor(private heroService: HeroService) { }

For each injected dependency, we’re getting the major issue:

Private properties that are only assigned in the constructor or at declaration should be “readonly”

Actually, the rule encourages us to code:

constructor(private readonly heroService: HeroService) { }

It would maybe be relevant to remove this detection for angular projects as it make code heavy and it’s not the way angular’s team wants us to inject dependency.

Hi,

I understand why this rule is not applicable to your project. In this case we suggest you to not activate the rule. It’s not part of default Sonar Way profile, so that’s ok to not have this rule executed.

Normally I’d say it’s a good indication to mark constants as readonly. But I agree that overly defensive programming is the reason why our code can become too heavy, and we should be able to have a better way to deal with such accidents. Especially if the Angular developers don’t find this style needed in their docs.

For example, in Java, there is a rule called " Utility classes should not have public constructors" (java:S1118), which forces heavy code on us as well, but at the same time there is also one called «Classes with only “static” methods should not be instantiated» (java:S2440), why don’t we just use that instead? I believe it shouldn’t be too hard to check whether someone accidentally reassigns @Injectable’s.

I know it’s quite hard to support all the quirks of all frameworks, but Angular is a very popular one, and SonarQube should be in sync with Angular on this rule, one way or an other. And by turning off the rule completely, we lose the ability to check for non-injected properties.

And finally, let me quote https://weblogs.asp.net/fredriknormen/defensive-programming-and-design-by-contract-on-a-routine-level :

When using a defensive driven programming too much, it could create problems. Too much validation codes could make the code less readable and affect performance, so don’t overuse defense driven programming. Defensive programming will make error easier to find, easier to fix, and less damage to production code. The result of using defense driven programming will in return increase the quality of the software.

1 Like