Skip to content

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

Closed
leongcmu wants to merge 103 commits into
mainfrom
jshint-integration
Closed

[P3A Documentation — Do Not Merge] JSHint Static Analysis Integration#153
leongcmu wants to merge 103 commits into
mainfrom
jshint-integration

Conversation

@leongcmu

@leongcmu leongcmu commented Mar 18, 2026

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: CMU-313#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 and others added 30 commits February 7, 2026 20:56
- Created createInstructorsGroup() function to initialize Instructors group during setup
- Created giveInstructorPrivileges() function to assign category and global privileges
- Instructors can now edit/delete posts, move topics, ban users, and purge content
- Group is visible in admin panel for manual member assignment
- Integrated both functions into install.setup() execution sequence
- Add 'endorsed' to intFields for proper integer parsing
- Initialize endorsed=0 on post creation
- Add endorse/unendorse API endpoints (PUT/DELETE /api/v3/posts/:pid/endorse)
- Add corresponding controller and route wiring

https://claude.ai/code/session_015H7tVc6LK7fwqr4tMk2psJ
…nd real-time updates

- Template: show endorse/unendorse toggle with icons, restrict to moderators
- API: add admin/mod privilege check, emit websocket events on endorse/unendorse
- Socket handler: include endorsed field in loadPostTools data
- Client JS: fix toggleEndorse to use data-endorsed attribute, remove debug logs
- Events: add onPostEndorsed handler for real-time UI updates across clients
- Theme: add endorsed indicator (check-circle) on post headers, endorsed CSS class
- Locales: add "endorsed" translation string

https://claude.ai/code/session_014eoruhm2UVyiTefaNqPBCx
- Created createInstructorsGroup() function to initialize Instructors group during setup
- Created giveInstructorPrivileges() function to assign category and global privileges
- Instructors can now edit/delete posts, move topics, ban users, and purge content
- Group is visible in admin panel for manual member assignment
- Integrated both functions into install.setup() execution sequence
…QKOH

Wire up endorse button end-to-end with UI toggle, privilege checks, a…
- Register 'posts:endorse' in category privilege map (moderation type)
- Add to topics.get() privilege check and returned privilege object
- Give default endorse privilege to admins/mods on new category creation
- Update API endorse/unendorse to use privileges.posts.can('posts:endorse')
- Add canEndorse check and display_endorse_tools flag in socket handler
- Set display_endorse_tools in modifyPostsByPrivilege from topic privileges
- Update template to gate endorse button on display_endorse_tools
- Add 'endorse-posts' admin UI label in en-US and en-GB locales

https://claude.ai/code/session_014eoruhm2UVyiTefaNqPBCx
…QKOH

Replace isAdminOrMod check with dedicated posts:endorse privilege
The endorse privilege was previously in the category privilege map, which
meant it only appeared when viewing a specific category's privileges in
the admin panel. Moving it to the global privilege map makes it visible
on the default admin privileges page under the Moderation tab.

https://claude.ai/code/session_014eoruhm2UVyiTefaNqPBCx
…QKOH

move post.endorse to global privilege
Add Instructor Role Support to User Management System
Refactor (src/user/posts.js:33): Reduce complexity for isReady()
…mail. Each user row now list their groups as badges. users.js, users.tpl, and users.json were edited.
emw868 and others added 26 commits February 27, 2026 19:48
…mation-Testing-for-U3

User Guide on Anonymous Feature updated, as well as automation tests in posts added.
Fix anonymous avatar default picture path. Updates to testing will be done.
…d-Testing-for-US1

Added automated tests and updated UserGuide.md
Add comments and documentations to the endorse functionalities
give NodeBB root privileges to the docker compose
Integrate JSHint Static Analysis Tool for JavaScript Code Quality Flagging
Add Stryker dynamic analysis evaluation
This reverts commit 62b0557, reversing
changes made to 9a5eea8.
@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.

@leongcmu leongcmu closed this Mar 18, 2026
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

5 participants