Resolve CodeQL extraction errors

[roslynwythe]

Overview

The CodeQL workflow GHA is reporting that it cannot scan 6 files in the gh-pages branch of hackforla/website due to extraction errors, presumably due to syntax errors. We need to resolve those errors to ensure complete testing coverage.

Action Items

• [x] Examine the CodeQL workflow logs in depth in order to understand the extraction and syntax errors.
• [x] Resolve the errors or advise dev leads/merge team why the errors cannot be resolved.

Resources/Instructions

GitHub CodeQL documentation. workflow run logs for codeql.yml

[github-actions[bot]]

Hi @elliot-d-kim, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

[elliot-d-kim]

Availability: Mon-Thu outside of 2-7pm, Fri before 2pm, Sat all day ETA: 2/13/24

[elliot-d-kim]

Progress Update

1. Progress: Discovered all 6 syntax errors raised by CodeQL appear to be related to YAML front matter in .js files under /assets/js.
2. Blockers: Planning to test CodeQL scan against changes in forked repo, but awaiting clarification from @roslynwythe on first-time set up of CodeQL for testing purposes.
3. Availability: Mon-Thu outside of 2-7pm, Fri before 2pm, Sat all day
4. ETA: 2/16/24

[roslynwythe]

@elliot-d-kim Please detail the files involved and whether they have liquid statements or just YAML front matter that is causing the problem with CodeQL. Thanks!

[elliot-d-kim]

"Syntax errors" prevent CodeQL from scanning .js files

It seems that the current CodeQL configuration can't scan JS files when YAML, Liquid, etc. is mixed in. The error message "Could not process some files due to syntax errors" sounds to me like these are not simply false alarms of code smells but rather indications that these "syntax errors" prevent CodeQL from scanning the 6 files involved.

Screenshot: CodeQL error message


The "syntax errors" are non-JS code

I believe the YAML and Liquid in these .js files are responsible. No other files in the .js directory have non-JS code, and CodeQL has no problem scanning those.

Summary: Non-JS code in these files

• hamburger-nav.js: YAML front-matter with a title
• toolkit.js: 1 line of Liquid, empty YAML front-matter
• wins.js : 2 lines (Liquid), empty YAML front-matter
• project.js : 2 lines (Liquid), empty YAML front-matter
• about.js: for loop (Liquid), empty YAML front-matter
• current-project.js: 2 lines + for loop (Liquid), empty YAML front-matter

Testing results

• Remove only the empty YAML front-matter: CodeQL errors moved down the files to the next non-JS (i.e. Liquid) lines.
• Remove all non-JS code (YAML, Liquid): CodeQL syntax errors resolve, files properly scanned

I did the latter in total 4 .js files: note that the message goes from "5 other warnings like this" (refer to the very first screenshot) to "1 other" in the following screenshot:

Screenshot: CodeQL error message


Deleting the Liquid lines would break the site (and CodeQL raised those errors accordingly in testing), so an alternative, holistic solution is required.

Deleting hamburger nav front-matter causes rendering errors in smaller views

Deleting the YAML front-matter in hamburger-nav.js made the file free of non-JS code and made the file completely free of CodeQL errors.

Errors in smaller views: Hamburger nav is not used in larger views but it appears in smaller or mobile views. This is mentioned in comments in _sass/components/_header-nav.scss under /* Hamburger nav CSS */ (line 83). Thus, while it causes no issues in larger views, deleting hamburger-nav.js's non-empty YAML front-matter causes a rendering error in smaller views.

Screenshot: Hamburger nav initial render

Screenshot: Hamburger nav interaction BEFORE deleting front-matter

Screenshot: Hamburger nav interaction AFTER deleting front-matter

[roslynwythe]

Great job @elliot-d-kim - your analysis is clear and compelling and supported by your testing. In future issues we will explore various options for solutions. Let me know if you would like to be involved in that work.

[elliot-d-kim]

Great, I'd definitely be interested in exploring solutions. I'm also curious about the progression of an issue like this up to solution implementation, so please keep me in the loop!

[roslynwythe]

• @elliot-d-kim For this situation we will require an epic. An epic is a higher-level issue that encompasses the "story" or requirements of multiple issues; that is, it is an issue to write and to track other related issues. I have started working on this: see #6378

If you are interested in working on the epic please contact me via Slack