mirror of
https://github.com/gnh1201/welsonjs.git
synced 2025-07-27 19:08:09 +00:00
updated pr-agent best practices with auto analysis
parent
01dd4d2fa4
commit
3931d8ace3
|
@ -1,22 +1,19 @@
|
||||||
<b>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.
|
<b>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.
|
||||||
</b>
|
</b>
|
||||||
|
|
||||||
Example code before:
|
Example code before:
|
||||||
```
|
```
|
||||||
public void ProcessData(Schema schema) {
|
const result = someObject.property;
|
||||||
var key = schema.PrimaryKey; // Risk of null reference
|
someRef.current.method();
|
||||||
this.features.join(','); // Risk if features is not array
|
|
||||||
}
|
|
||||||
```
|
```
|
||||||
|
|
||||||
Example code after:
|
Example code after:
|
||||||
```
|
```
|
||||||
public void ProcessData(Schema schema) {
|
if (someObject && someObject.property) {
|
||||||
if (schema == null) throw new ArgumentNullException(nameof(schema));
|
const result = someObject.property;
|
||||||
var key = schema.PrimaryKey;
|
}
|
||||||
if (Array.isArray(this.features) && this.features.length > 0) {
|
if (someRef.current) {
|
||||||
this.features.join(',');
|
someRef.current.method();
|
||||||
}
|
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
<details><summary>Relevant past accepted suggestions:</summary>
|
<details><summary>Relevant past accepted suggestions:</summary>
|
||||||
|
@ -106,19 +103,21 @@ Add null check
|
||||||
___
|
___
|
||||||
|
|
||||||
|
|
||||||
<b>Pattern 2: Validate string length and bounds before performing substring operations or string manipulations to prevent ArgumentOutOfRangeException and ensure safe string processing.
|
<b>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.
|
||||||
</b>
|
</b>
|
||||||
|
|
||||||
Example code before:
|
Example code before:
|
||||||
```
|
```
|
||||||
string result = path.Substring("prefix/".Length);
|
const parsed = JSON.parse(response);
|
||||||
bool isValid = Regex.IsMatch(path, @"^[^/@]+@[^/]+/");
|
cmd.push("--disable-features=" + features.join(','));
|
||||||
```
|
```
|
||||||
|
|
||||||
Example code after:
|
Example code after:
|
||||||
```
|
```
|
||||||
string result = path.Length > "prefix/".Length ? path.Substring("prefix/".Length) : path;
|
const parsed = typeof response === 'string' ? JSON.parse(response) : response;
|
||||||
bool isValid = Regex.IsMatch(path, @"^(@[a-z0-9-]+\/[a-z0-9-]+|[a-z0-9-]+)@([0-9]+\.[0-9]+\.[0-9]+|latest|[^/]+)/");
|
if (Array.isArray(features) && features.length > 0) {
|
||||||
|
cmd.push("--disable-features=" + features.join(','));
|
||||||
|
}
|
||||||
```
|
```
|
||||||
<details><summary>Relevant past accepted suggestions:</summary>
|
<details><summary>Relevant past accepted suggestions:</summary>
|
||||||
|
|
||||||
|
@ -128,29 +127,19 @@ ___
|
||||||
<b>
|
<b>
|
||||||
Suggestion 1</b>:
|
Suggestion 1</b>:
|
||||||
|
|
||||||
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
|
```diff
|
||||||
private async Task<bool> TryServeFromCdn(HttpListenerContext context, string path)
|
-// disable an unwanted features
|
||||||
{
|
-cmd.push("--disable-features=" + this.disableFeatures.join(','));
|
||||||
bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/");
|
+// disable unwanted features
|
||||||
|
+if (Array.isArray(this.disableFeatures) && this.disableFeatures.length > 0) {
|
||||||
var sources = new (bool isMatch, string configKey, Func<string, string> transform)[]
|
+ cmd.push("--disable-features=" + this.disableFeatures.join(','));
|
||||||
{
|
+}
|
||||||
(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
|
|
||||||
};
|
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
@ -160,15 +149,15 @@ ___
|
||||||
<b>
|
<b>
|
||||||
Suggestion 2</b>:
|
Suggestion 2</b>:
|
||||||
|
|
||||||
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
|
```diff
|
||||||
-bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/");
|
-response = JSON.parse(response)
|
||||||
+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 = 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]`
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue
Block a user