From 3931d8ace3571df6664e23ed1b80779d7f13e3ad Mon Sep 17 00:00:00 2001 From: root Date: Sat, 26 Jul 2025 14:20:23 +0000 Subject: [PATCH] updated pr-agent best practices with auto analysis --- .pr_agent_auto_best_practices.md | 71 ++++++++++++++------------------ 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/.pr_agent_auto_best_practices.md b/.pr_agent_auto_best_practices.md index a7bac28..efbf2aa 100644 --- a/.pr_agent_auto_best_practices.md +++ b/.pr_agent_auto_best_practices.md @@ -1,22 +1,19 @@ -Pattern 1: Add null checks and validation before accessing object properties or calling methods to prevent null reference exceptions and runtime errors. This includes checking if objects, arrays, or references exist before using them. +Pattern 1: Add null/undefined checks before accessing object properties or calling methods to prevent runtime exceptions. This includes checking if variables, object references, or array elements exist before performing operations on them. Example code before: ``` -public void ProcessData(Schema schema) { - var key = schema.PrimaryKey; // Risk of null reference - this.features.join(','); // Risk if features is not array -} +const result = someObject.property; +someRef.current.method(); ``` Example code after: ``` -public void ProcessData(Schema schema) { - if (schema == null) throw new ArgumentNullException(nameof(schema)); - var key = schema.PrimaryKey; - if (Array.isArray(this.features) && this.features.length > 0) { - this.features.join(','); - } +if (someObject && someObject.property) { + const result = someObject.property; +} +if (someRef.current) { + someRef.current.method(); } ```
Relevant past accepted suggestions: @@ -106,19 +103,21 @@ Add null check ___ -Pattern 2: Validate string length and bounds before performing substring operations or string manipulations to prevent ArgumentOutOfRangeException and ensure safe string processing. +Pattern 2: Add type checks before performing operations that assume a specific data type, such as parsing JSON strings or calling array methods. Verify the data type matches expectations before proceeding with type-specific operations. Example code before: ``` -string result = path.Substring("prefix/".Length); -bool isValid = Regex.IsMatch(path, @"^[^/@]+@[^/]+/"); +const parsed = JSON.parse(response); +cmd.push("--disable-features=" + features.join(',')); ``` Example code after: ``` -string result = path.Length > "prefix/".Length ? path.Substring("prefix/".Length) : path; -bool isValid = Regex.IsMatch(path, @"^(@[a-z0-9-]+\/[a-z0-9-]+|[a-z0-9-]+)@([0-9]+\.[0-9]+\.[0-9]+|latest|[^/]+)/"); +const parsed = typeof response === 'string' ? JSON.parse(response) : response; +if (Array.isArray(features) && features.length > 0) { + cmd.push("--disable-features=" + features.join(',')); +} ```
Relevant past accepted suggestions: @@ -128,29 +127,19 @@ ___ Suggestion 1: -Prevent substring exception +Fix grammar and add validation -**The JqueryCdnPrefix transform is removing the "jquery/" prefix from the path, but this could cause issues if the path doesn't have sufficient length. Add a check to prevent potential ArgumentOutOfRangeException when the path is too short.** +**The comment contains a grammatical error ("an unwanted features" should be "unwanted features"). Additionally, consider adding validation to ensure disableFeatures is an array before calling join() to prevent runtime errors.** -[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [196-210]](https://github.com/gnh1201/welsonjs/pull/245/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R196-R210) +[lib/chrome.js [323-324]](https://github.com/gnh1201/welsonjs/pull/276/files#diff-868ee66369a322a75a321ede18241bfb18f199a9e5bb5371d4d049f0af14f598R323-R324) ```diff - private async Task TryServeFromCdn(HttpListenerContext context, string path) - { - bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/"); - - var sources = new (bool isMatch, string configKey, Func transform)[] - { - (path.StartsWith("ajax/libs/"), "CdnJsPrefix", p => p), - (isNodePackageExpression, "UnpkgPrefix", p => p), - (isNodePackageExpression, "SkypackPrefix", p => p), - (isNodePackageExpression, "EsmShPrefix", p => p), - (isNodePackageExpression, "EsmRunPrefix", p => p), - (path.StartsWith("npm/") || path.StartsWith("gh/") || path.StartsWith("wp/"), "JsDeliverPrefix", p => p), -- (path.StartsWith("jquery/"), "JqueryCdnPrefix", p => p.Substring("jquery/".Length)), -+ (path.StartsWith("jquery/") && path.Length > "jquery/".Length, "JqueryCdnPrefix", p => p.Substring("jquery/".Length)), - (true, "BlobStoragePrefix", p => p) // fallback - }; +-// disable an unwanted features +-cmd.push("--disable-features=" + this.disableFeatures.join(',')); ++// disable unwanted features ++if (Array.isArray(this.disableFeatures) && this.disableFeatures.length > 0) { ++ cmd.push("--disable-features=" + this.disableFeatures.join(',')); ++} ``` @@ -160,15 +149,15 @@ ___ Suggestion 2: -Improve package detection regex +Add type check before parsing -**The regex pattern for detecting node package expressions might not handle all valid package formats. Consider using a more comprehensive pattern that accounts for scoped packages (starting with @) and various version formats.** +**The code assumes the response is always a string that needs parsing, but HTTP responses might already be parsed objects. This could cause errors if the response is already a JSON object. Add a type check before parsing.** -[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [198]](https://github.com/gnh1201/welsonjs/pull/245/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R198-R198) +[lib/language-inference-engine.js [217]](https://github.com/gnh1201/welsonjs/pull/248/files#diff-2ab61534138b13248e897d3b1aac2dbe3d8bd44ade898729c432d822f1623a58R217-R217) ```diff --bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/"); -+bool isNodePackageExpression = Regex.IsMatch(path, @"^(@[a-z0-9-]+\/[a-z0-9-]+|[a-z0-9-]+)@([0-9]+\.[0-9]+\.[0-9]+|[0-9]+\.[0-9]+|[0-9]+|latest|next|[^/]+)/"); +-response = JSON.parse(response) ++response = typeof response === 'string' ? JSON.parse(response) : response; ``` @@ -179,5 +168,5 @@ Improve package detection regex ___ -`[Auto-generated best practices - 2025-06-26]` +`[Auto-generated best practices - 2025-07-26]`