From f797a72fd36ca3f18017a4b6370c497d37842615 Mon Sep 17 00:00:00 2001 From: kaufco Date: Fri, 31 Jan 2025 16:38:02 +0000 Subject: [PATCH 1/2] Create rule S7197 --- rules/S7197/javascript/metadata.json | 25 ++++++++++++++++ rules/S7197/javascript/rule.adoc | 44 ++++++++++++++++++++++++++++ rules/S7197/metadata.json | 2 ++ 3 files changed, 71 insertions(+) create mode 100644 rules/S7197/javascript/metadata.json create mode 100644 rules/S7197/javascript/rule.adoc create mode 100644 rules/S7197/metadata.json diff --git a/rules/S7197/javascript/metadata.json b/rules/S7197/javascript/metadata.json new file mode 100644 index 00000000000..0911f50b554 --- /dev/null +++ b/rules/S7197/javascript/metadata.json @@ -0,0 +1,25 @@ +{ + "title": "FIXME", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-7197", + "sqKey": "S7197", + "scope": "All", + "defaultQualityProfiles": ["Sonar way"], + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "HIGH", + "RELIABILITY": "MEDIUM", + "SECURITY": "LOW" + }, + "attribute": "CONVENTIONAL" + } +} diff --git a/rules/S7197/javascript/rule.adoc b/rules/S7197/javascript/rule.adoc new file mode 100644 index 00000000000..dba2b9fc997 --- /dev/null +++ b/rules/S7197/javascript/rule.adoc @@ -0,0 +1,44 @@ +FIXME: add a description + +// If you want to factorize the description uncomment the following line and create the file. +//include::../description.adoc[] + +== Why is this an issue? + +FIXME: remove the unused optional headers (that are commented out) + +//=== What is the potential impact? + +== How to fix it +//== How to fix it in FRAMEWORK NAME + +=== Code examples + +==== Noncompliant code example + +[source,javascript,diff-id=1,diff-type=noncompliant] +---- +FIXME +---- + +==== Compliant solution + +[source,javascript,diff-id=1,diff-type=compliant] +---- +FIXME +---- + +//=== How does this work? + +//=== Pitfalls + +//=== Going the extra mile + + +//== Resources +//=== Documentation +//=== Articles & blog posts +//=== Conference presentations +//=== Standards +//=== External coding guidelines +//=== Benchmarks diff --git a/rules/S7197/metadata.json b/rules/S7197/metadata.json new file mode 100644 index 00000000000..2c63c085104 --- /dev/null +++ b/rules/S7197/metadata.json @@ -0,0 +1,2 @@ +{ +} From fc7ec2fc93217686d6192260323bb507ade02c76 Mon Sep 17 00:00:00 2001 From: Marco Kaufmann Date: Fri, 31 Jan 2025 18:25:48 +0100 Subject: [PATCH 2/2] Create rule S7197: Circular file imports should be resolved --- rules/S7197/javascript/metadata.json | 14 ++-- rules/S7197/javascript/rule.adoc | 96 ++++++++++++++++++++++------ 2 files changed, 83 insertions(+), 27 deletions(-) diff --git a/rules/S7197/javascript/metadata.json b/rules/S7197/javascript/metadata.json index 0911f50b554..7021e60b7a7 100644 --- a/rules/S7197/javascript/metadata.json +++ b/rules/S7197/javascript/metadata.json @@ -1,25 +1,25 @@ { - "title": "FIXME", + "title": "Circular file imports should be resolved", "type": "CODE_SMELL", "status": "ready", "remediation": { "func": "Constant\/Issue", - "constantCost": "5min" + "constantCost": "0min" }, "tags": [ + "architecture", + "design" ], "defaultSeverity": "Major", "ruleSpecification": "RSPEC-7197", "sqKey": "S7197", "scope": "All", "defaultQualityProfiles": ["Sonar way"], - "quickfix": "unknown", + "quickfix": "infeasible", "code": { "impacts": { - "MAINTAINABILITY": "HIGH", - "RELIABILITY": "MEDIUM", - "SECURITY": "LOW" + "MAINTAINABILITY": "HIGH" }, - "attribute": "CONVENTIONAL" + "attribute": "MODULAR" } } diff --git a/rules/S7197/javascript/rule.adoc b/rules/S7197/javascript/rule.adoc index dba2b9fc997..49cc32b543e 100644 --- a/rules/S7197/javascript/rule.adoc +++ b/rules/S7197/javascript/rule.adoc @@ -1,44 +1,100 @@ -FIXME: add a description - -// If you want to factorize the description uncomment the following line and create the file. -//include::../description.adoc[] +This rule reports circular dependencies between source files caused by circular imports. == Why is this an issue? -FIXME: remove the unused optional headers (that are commented out) +Circular dependencies occur when two or more source files import each other, either directly or indirectly. +This creates a dependency structure that lacks a clear hierarchy, making the codebase harder to understand and maintain. +Additionally, the order in which circular imports are resolved is not guaranteed, which can lead to unpredictable behavior and runtime errors. + +=== What is the potential impact? -//=== What is the potential impact? +Circular dependencies increase the complexity of the code architecture, reducing readability, extensibility, and maintainability. +As the project grows, these dependencies can spread, further complicating the architecture and increasing technical debt. +Over time, resolving these dependencies becomes increasingly difficult. == How to fix it -//== How to fix it in FRAMEWORK NAME + +1. **Refactor shared functionality**: If multiple files share similar functionality, consider moving that functionality to a separate module that both can import. This allows each file to depend on the shared module rather than on each other. + +2. **Use dependency inversion:** Instead of directly importing modules that create circular dependencies, use dependency inversion by passing necessary functions or objects as parameters. This breaks the circular reference and makes the code more modular and testable. + +3. **Split responsibilities**: Evaluate whether each file is handling too many responsibilities. If so, break them down into smaller, more focused modules. This reduces circular dependencies and ensures that your code is easier to manage and extend. === Code examples ==== Noncompliant code example +The following code contains a circular dependency: `order.js` → `customer.js` → `order.js`, and `order.js` → `product.js` → `order.js`. +Both cycles are connected through `order.js`, forming a tangle of two cycles. + [source,javascript,diff-id=1,diff-type=noncompliant] ---- -FIXME +// order.js +import { Customer } from './customer.js'; +import { Product } from './product.js'; + +export class Order { + constructor() { + this.customer = new Customer(); + this.products = []; + } +} + +// customer.js +import { Order } from './order.js'; + +export class Customer { + constructor() { + this.orders = []; + } +} + +// product.js +import { Order } from './order.js'; + +export class Product { + constructor() { + this.orders = []; + } +} ---- ==== Compliant solution +The issue can be resolved by refactoring the structure. +Two service functions can replace the dependencies `customer.js` → `order.js` and `product.js` → `order.js`. + [source,javascript,diff-id=1,diff-type=compliant] ---- -FIXME ----- +// order.js +import { Customer } from './customer.js'; +import { Product } from './product.js'; + +export class Order { + constructor() { + this.customer = new Customer(); + this.products = []; + } +} -//=== How does this work? +// customer.js +export class Customer { } -//=== Pitfalls +// product.js +export class Product { } -//=== Going the extra mile +// orderService.js +export function getOrdersByCustomer(customer) { + // Implementation to get orders by customer +} + +export function getOrdersByProduct(product) { + // Implementation to get orders by product +} +---- +== Resources -//== Resources -//=== Documentation -//=== Articles & blog posts -//=== Conference presentations -//=== Standards -//=== External coding guidelines -//=== Benchmarks +- Wikipedia - https://en.wikipedia.org/wiki/Acyclic_dependencies_principle[Acyclic dependencies principle] +- STAN - https://stan4j.com/advanced/adp/[Acyclic dependencies principle] +- RSPEC - https://sonarsource.github.io/rspec/#/rspec/S7091/java[S7091: Circular dependencies between classes across package boundaries should be resolved]