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

Resolve JS-3 Stricter rule implementation for detecting a valid header #4662

Merged
merged 17 commits into from
Apr 18, 2024

Conversation

zglicz
Copy link
Contributor

@zglicz zglicz commented Apr 16, 2024

Trying to expand on the logic in https://github.com/SonarSource/sonar-html/blob/master/sonar-html-plugin/src/main/java/org/sonar/plugins/html/checks/sonar/TableWithoutHeaderCheck.java

Trying to be stricter about the implementation and the logic applied is as follows:

  1. Recursively read the contents of a (skipping nested <table>'s)
  2. Expand the colspan's
  3. Expand the rowspan's and build the full table "grid"
  4. Check that there exists any row or any column that is a header.
  5. For the most part following the spec - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/th
    The only missing thing is not breaking up separately the <thead> , <tbody> and <tfoot> and running the full grid check.

Copy link

sonarqube-next bot commented Apr 18, 2024

Copy link
Contributor

@vdiez vdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

const rows: TableCell[][] = [];
let unknownTableStructure = false;
tree.children.forEach(child => {
if (child.type === 'JSXElement') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reminder: handle JSXFragment

@vdiez vdiez merged commit 657055c into master Apr 18, 2024
16 checks passed
@vdiez vdiez deleted the js-3 branch April 18, 2024 15:04
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot assigned zglicz and unassigned vdiez Apr 18, 2024
@vdiez vdiez added this to the 10.14.0 milestone Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants