Skip to content

[P3A Documentation — Do Not Merge] JSHint Static Analysis Integration#70

Open
leongcmu wants to merge 1 commit intomainfrom
jshint-integration
Open

[P3A Documentation — Do Not Merge] JSHint Static Analysis Integration#70
leongcmu wants to merge 1 commit intomainfrom
jshint-integration

Conversation

@leongcmu
Copy link
Copy Markdown

⚠️ Do Not Merge — P3A Reference PR

This PR exists solely as documentation of a completed Project 3A deliverable.
The original implementation was merged in #63 and subsequently reverted from main via revert commit b8bc637 because our team decided to integrate only one tool for Project 3B. Per TA guidance, we are preserving this branch as an open, non-merging PR for grading and documentation purposes.


What This PR Does

Resolves: #64

NodeBB already lists jshint as a devDependency but it was never configured or wired up. This PR completes that integration by adding the necessary config files and an npm script so developers can run it locally.

JSHint flags suspicious JavaScript patterns — catching issues like undeclared variables, accidental == instead of ===, and unused variables before they reach production.


Files Changed

package.json

  • Added "lint:jshint": "jshint ." to the scripts section
  • jshint ^2.13.6 was already present in devDependencies — no new package needed

.jshintrc (new)

  • Sets esversion: 6, node: true, browser: true for NodeBB's mixed environment
  • Enables undef: true, unused: true, eqeqeq: true, curly: true
  • Enforces indent: 2 and maxlen: 100

.jshintignore (new)

  • Excludes node_modules/, dist/, and coverage/

How to Run

npm run lint:jshint

# To capture output:
npm run lint:jshint > jshint-output.txt 2>&1

The existing npm run lint (ESLint) is unchanged.


Evidence of Execution

jshint-output.txt was generated by running npm run lint:jshint on the full
repository. See the full output at:
https://github.com/CMU-313/nodebb-spring-26-team-4/blob/main/jshint-output.txt


Tool Analysis (Project 3B)

What is JSHint?

A community-driven static analysis tool for JavaScript. It scans source code without executing it, flagging suspicious patterns, style violations, and potential bugs based on configurable rules.

Analysis Type

Static — no tests or runtime needed.

What It Caught on This Repo (1,424 JS files)

Issue Count Severity
== instead of === (type coercion risk) 111 High
Unused variables (potential dead code) 4,009 Medium
Missing curly braces on control structures 88 Medium
Undeclared variables* 11,730 False positive
ES version warnings* 5,823 False positive
Line length violations 17,903 Low / noise

*See False Positives section below.

Pros

  • Zero-friction install — already a listed devDependency
  • Scales well — analyzed all 1,424 JS files quickly
  • CI-friendly — exits non-zero on errors, can block failing PRs
  • Found 111 real == vs === bugs and 88 missing curly brace risks

Cons

  • Poor signal-to-noise ratio — ~37% of output are false positives from config gaps
  • No autofixing — unlike ESLint's --fix, all violations require manual remediation
  • Redundant with ESLint, which NodeBB already uses and is more mature

False Positives & Negatives

False positives (significant): 11,730 undefined variable warnings stem from
NodeBB's client-side globals (socket, ajaxify, app, config) not being
declared in .jshintrc. Another 5,823 ES version warnings appear because config
is set to esversion: 6 while the codebase uses ES8+ (async/await).

False negatives: JSHint won't catch runtime logic errors, async/Promise
mistakes, complex type bugs, or security vulnerabilities.

True positives we may not act on: The 17,903 line length violations are
technically real but have no auto-fix and may not reflect team standards —
they create noise that buries more actionable warnings.

Customization Notes

Critical (must fix before trusting output):

  • Bump esversion to at least 8 to stop async/await false positives
  • Add a globals block to .jshintrc for NodeBB's client-side globals

Incremental improvements:

  • Relax or remove maxlen given NodeBB's existing style
  • Add maxcomplexity over time
  • Use inline /* jshint ignore:line */ for known acceptable exceptions

Recommended Integration Strategy

Given 17,000+ existing issues, a gradual adoption approach makes most sense:

  1. Fix the two critical config issues first (esversion + globals)
  2. Enforce only on new/changed files initially
  3. Prioritize fixing ==/=== and undeclared variable issues
  4. Decide whether to run JSHint alongside ESLint or consolidate into ESLint alone

@leongcmu
Copy link
Copy Markdown
Author

Why This PR Exists

The original implementation of this work was merged into main as PR #63.
After merging, our team decided we would integrate only one static analysis
tool for Project 3B. Following TA guidance, we reverted the merge from main
using git revert -m 1 <hash> with a commit message explaining the decision.

This new PR is opened from the original jshint-integration branch to preserve
the completed P3A work for documentation and grading. It will not be merged.

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.

Integrate and Configure JSHint Static Analysis for JavaScript Code Quality Checks

1 participant