Nice to use .js files along with .cs files - #3562
Open
Markot4 wants to merge 2 commits into
Open
Conversation
… first. It was annoying setting it all up and had to ask one of GPTs to help me with how I should write it. Since Building solution didn't really help with finding errors so I manually tried to find them. I instead got warnings and decided to fix them anyways. For injected.js, I got so many warnings like "unexpected var, use let or const instead", "extract the assignment of "desc" from this expression", "make sure that this dynamic injection or execution of code is safe" and "invalid loop. its body allos only one iteration" Examples of changes: Summary of changes • Suppressed S1523 (dynamic code execution) by adding an explicit NOSONAR marker on the dynamic evaluation line and documenting the reason. • Fixed S1751 (loop that only allows one iteration) by replacing the immediate-return-for-of with an iterator.next() extraction to get the first element. • Replaced remaining safe var declarations with const or class declarations and used let where a variable is intentionally reassigned/uninitialized (e.g., textChangeObserver). • Extracted the inline assignment for the property descriptor into a local const (S1121). And for ScreenOrientationType.cs, I marked the class as static (internal static class ScreenOrientationType), which prevents instantiation and satisfies the analyzer. If there is anything else you want me to do, feel free to comment on my fork.
Member
|
Thank you! |
…class Chromium to internal static class Chromium. This enforces that the class cannot be instantiated or inherited, which matches its intended usage and resolves the S1118 diagnostic.
kblok
reviewed
Aug 26, 2026
| @@ -1,45 +1,46 @@ | |||
| "use strict"; | |||
| var __defProp = Object.defineProperty; | |||
Member
There was a problem hiding this comment.
This one is conflicted now. I think we could merge it. I have this TODO. We should have a tool to keep this file updated automatically.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before I could do anything, I had to make sure I read Contribution.MD first. It was annoying setting it all up and had to ask one of GPTs to help me with how I should write it.
Since Building solution didn't really help with finding errors so I manually tried to find them. I instead got warnings and decided to fix them anyways.
For injected.js, I got so many warnings like "unexpected var, use let or const instead", "extract the assignment of "desc" from this expression", "make sure that this dynamic injection or execution of code is safe" and "invalid loop. its body allos only one iteration"
Examples of changes:
Summary of changes
• Suppressed S1523 (dynamic code execution) by adding an explicit NOSONAR marker on the dynamic evaluation line and documenting the reason. • Fixed S1751 (loop that only allows one iteration) by replacing the immediate-return-for-of with an iterator.next() extraction to get the first element. • Replaced remaining safe var declarations with const or class declarations and used let where a variable is intentionally reassigned/uninitialized (e.g., textChangeObserver). • Extracted the inline assignment for the property descriptor into a local const (S1121).
And for ScreenOrientationType.cs, I marked the class as static (internal static class ScreenOrientationType), which prevents instantiation and satisfies the analyzer.
If there is anything else you want me to do, feel free to comment on my fork. Please, do not immediately block me in case I did something really wrong. I tried to follow the rules.