-
Notifications
You must be signed in to change notification settings - Fork 107
Smar 49 create ai conventions checker git action #1623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
purriza
wants to merge
25
commits into
main
Choose a base branch
from
SMAR-49-Create-AI-conventions-checker-git-action
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
b67d1f9
Added new GitHub action, check conventions and guidelines
purriza 88e7e4c
Removed GitHub action to check conventions
purriza ef1fcfc
test: Add Solidity contract violations for CodeRabbit testing
purriza afb68fc
refactor: organize CodeRabbit violations by category and add deployme…
purriza ebf06ab
fix: move deployment script violations to script/deploy/ for CodeRabb…
purriza a6bc51f
fix: move Solidity violations to src/ for CodeRabbit detection
purriza e16776c
fix: resolve security warnings in violation examples
purriza 1a20495
feat: add TypeScript violation examples for CodeRabbit validation
purriza 72d67b8
fix: improve TypeScript violation examples for better CodeRabbit dete…
purriza f4d1c89
fix: update BadCodeQuality and BadImportOrder with improved violations
purriza bde5903
Merge branch 'main' into SMAR-49-Create-AI-conventions-checker-git-ac…
purriza 21b3949
Removed TypeScript violation examples. Added bash examples
purriza 61729ac
Added Solidity violation file
purriza f958a82
Removed solidity violation file
purriza 8f34c9a
Added violation test files
purriza 1e74227
Removed scripts
purriza 949c271
Merge branch 'main' into SMAR-49-Create-AI-conventions-checker-git-ac…
purriza 94bd7a8
Added workflow violation file
purriza 5b3b1af
Added .sh violation files
purriza b6c1b02
Added .cursorrules for Coderabbit
purriza c0dbe42
Removed script violations and added .sol
purriza 1ea00ba
Merge branch 'main' into SMAR-49-Create-AI-conventions-checker-git-ac…
purriza cf8c767
Solidity violation files moved
purriza fad4d82
Removed .sol and added .t.sol violations
purriza acc9bd9
Updated test to force violations
purriza File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /** | ||
| * Violation: Code quality issues - uses `any`, poor error handling. | ||
| * | ||
| * Convention violations: | ||
| * - Obey .eslintrc.cjs; avoid `any` types | ||
| * - Use proper error handling with consola.error() | ||
| * - Exit with process.exit(1) on fatal errors | ||
| * - Provide meaningful error messages | ||
| * | ||
| * This file violates by using `any` types and not handling errors properly. | ||
| */ | ||
|
|
||
| import { consola } from 'consola' | ||
|
|
||
| // Violation: Uses `any` type instead of proper typing | ||
| function processData(data: any) { | ||
| // Violation: Should use proper types (unknown with type narrowing, or specific interface) | ||
| return data.value | ||
| } | ||
|
|
||
| // Violation: Uses `any` for error handling | ||
| function handleError(error: any) { | ||
| // Violation: Should use proper error type (Error | unknown with narrowing) | ||
| console.log(error.message) // Violation: Uses console.log instead of consola | ||
| } | ||
|
|
||
| // Violation: Doesn't exit with process.exit(1) on fatal errors | ||
| function fatalOperation() { | ||
| try { | ||
| // Some operation that might fail | ||
| throw new Error('Fatal error') | ||
| } catch (error) { | ||
| // Violation: Should use consola.error() and process.exit(1) | ||
| console.error('Error occurred') // Violation: Uses console.error instead of consola.error | ||
| // Missing: process.exit(1) | ||
| } | ||
| } | ||
|
|
||
| // Violation: Poor error messages | ||
| function badFunction() { | ||
| try { | ||
| // Operation | ||
| } catch { | ||
| // Violation: No error message, should provide meaningful message | ||
| throw new Error('Error') // Violation: Generic error message | ||
| } | ||
| } | ||
|
|
||
| // Violation: Uses `any` for function parameters | ||
| function processContract(contract: any) { | ||
| // Violation: Should use proper type (e.g., from typechain or viem) | ||
| return contract.address | ||
| } | ||
|
|
||
| export { processData, handleError, fatalOperation, badFunction, processContract } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /** | ||
| * Violation: Duplicates code that should use existing helpers. | ||
| * | ||
| * Convention violation: Check existing helpers in: | ||
| * - script/common/ | ||
| * - script/utils/ | ||
| * - script/demoScripts/utils/ | ||
| * | ||
| * Key helpers: deploymentHelpers.ts, demoScriptHelpers.ts, script/common/types.ts | ||
| * | ||
| * This file violates by re-implementing logic that exists in helpers. | ||
| */ | ||
|
|
||
| import { consola } from 'consola' | ||
| import { createPublicClient, http, getAddress, type Address } from 'viem' | ||
|
|
||
| // Violation: Re-implements network config reading that exists in viemScriptHelpers | ||
| function getNetworkConfig(networkName: string) { | ||
| // Violation: Should use getViemChainForNetworkName from viemScriptHelpers | ||
| const networks = require('../../config/networks.json') | ||
| return networks[networkName] | ||
| } | ||
|
|
||
| // Violation: Re-implements deployment address reading that exists in deploymentHelpers | ||
| function getDeploymentAddress(network: string, contractName: string) { | ||
| // Violation: Should use getDeployments from deploymentHelpers | ||
| const deployments = require(`../../deployments/${network}.json`) | ||
| return deployments[contractName] | ||
| } | ||
|
|
||
| // Violation: Re-implements address validation that exists in helpers | ||
| function validateAddress(address: string): Address { | ||
| // Violation: Should use getAddress from viem (already imported but re-implemented) | ||
| if (!address.startsWith('0x') || address.length !== 42) { | ||
| throw new Error('Invalid address') | ||
| } | ||
| return address as Address | ||
| } | ||
|
|
||
| // Violation: Re-implements client creation pattern that exists in demoScriptHelpers | ||
| function createClient(networkName: string) { | ||
| // Violation: Should use helpers from demoScriptHelpers or viemScriptHelpers | ||
| const network = getNetworkConfig(networkName) | ||
| return createPublicClient({ | ||
| transport: http(network.rpcUrl), | ||
| }) | ||
| } | ||
|
|
||
| export async function badFunction() { | ||
| // Uses duplicated logic instead of helpers | ||
| const network = getNetworkConfig('mainnet') | ||
| const address = getDeploymentAddress('mainnet', 'LiFiDiamond') | ||
| const client = createClient('mainnet') | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /** | ||
| * Violation: Uses deprecated ethers.js helpers instead of viem. | ||
| * | ||
| * Convention violation: Contract interactions MUST use viem. | ||
| * DO NOT use deprecated ethers.js helpers like: | ||
| * - getProvider | ||
| * - getWalletFromPrivateKeyInDotEnv | ||
| * - ethers sendTransaction | ||
| * - ensureBalanceAndAllowanceToDiamond | ||
| * | ||
| * This file violates by using ethers.js for contract interactions. | ||
| */ | ||
|
|
||
| import { ethers } from 'ethers' | ||
| import { consola } from 'consola' | ||
|
|
||
| // Violation: Uses deprecated ethers.js getProvider helper | ||
| function getProvider(network: string) { | ||
| // Violation: Should use viem's createPublicClient instead | ||
| return ethers.getDefaultProvider(network) | ||
| } | ||
|
|
||
| // Violation: Uses deprecated ethers.js getWalletFromPrivateKeyInDotEnv | ||
| function getWallet() { | ||
| // Violation: Should use viem's privateKeyToAccount instead | ||
| const privateKey = process.env.PRIVATE_KEY || '' | ||
| return new ethers.Wallet(privateKey, getProvider('mainnet')) | ||
| } | ||
|
|
||
| // Violation: Uses ethers.js sendTransaction instead of viem | ||
| async function sendTransaction() { | ||
| const wallet = getWallet() | ||
| const contract = new ethers.Contract('0x...', ['function transfer()'], wallet) | ||
|
|
||
| // Violation: Should use viem's writeContract or sendTransaction | ||
| await contract.transfer() | ||
| } | ||
|
|
||
| // Violation: Uses deprecated ensureBalanceAndAllowanceToDiamond helper | ||
| async function ensureBalance() { | ||
| // Violation: Should use viem-based helpers from deploymentHelpers or demoScriptHelpers | ||
| // This function doesn't exist in ethers.js, but represents deprecated pattern | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /** | ||
| * Violation: Imports are in incorrect order. | ||
| * | ||
| * Convention violation: Imports MUST be ordered as: | ||
| * 1. External libs (viem, consola, citty, dotenv) | ||
| * 2. TypeChain types (typechain/) | ||
| * 3. Config files | ||
| * 4. Internal utils/helpers | ||
| * | ||
| * This file violates by importing internal utils before external libs. | ||
| */ | ||
|
|
||
| // Violation: Internal utils imported before external libs | ||
| import { getDeployments } from '../utils/deploymentHelpers' | ||
| import { getViemChainForNetworkName } from '../utils/viemScriptHelpers' | ||
| import type { SupportedChain } from '../common/types' | ||
|
|
||
| // Violation: External libs imported after internal utils | ||
| import { consola } from 'consola' | ||
| import { createPublicClient, http, type Address } from 'viem' | ||
|
|
||
| // Violation: Config imported after external libs (should be before internal utils) | ||
| import networksConfig from '../../config/networks.json' | ||
|
|
||
| // Violation: TypeChain types imported last (should be second, after external libs) | ||
| import type { ILiFi } from '../../../typechain' | ||
|
|
||
| export async function badFunction() { | ||
| // Function implementation... | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /** | ||
| * Violation: Safe decode script doesn't use formatDecodedTxDataForDisplay. | ||
| * | ||
| * Convention violation: Human-readable decoded Safe/timelock transaction data | ||
| * MUST be produced via formatDecodedTxDataForDisplay(data, context) from | ||
| * safe-decode-utils.ts. | ||
| * | ||
| * Scripts showing Safe or timelock calldata MUST call this function instead | ||
| * of duplicating decode/format logic. | ||
| * | ||
| * This file violates by implementing its own decode/format logic. | ||
| */ | ||
|
|
||
| import { consola } from 'consola' | ||
| import { decodeFunctionData, parseAbi, type Hex } from 'viem' | ||
|
|
||
| // Violation: Implements custom decode logic instead of using formatDecodedTxDataForDisplay | ||
| function decodeSafeTransaction(data: Hex) { | ||
| // Violation: Should use formatDecodedTxDataForDisplay from safe-decode-utils.ts | ||
| const abi = parseAbi(['function diamondCut(...)']) | ||
|
|
||
| try { | ||
| const decoded = decodeFunctionData({ | ||
| abi, | ||
| data, | ||
| }) | ||
|
|
||
| // Violation: Custom formatting instead of using formatDecodedTxDataForDisplay | ||
| consola.info('Decoded transaction:') | ||
| consola.info(`Function: ${decoded.functionName}`) | ||
| consola.info(`Args: ${JSON.stringify(decoded.args)}`) | ||
|
|
||
| return decoded | ||
| } catch (error) { | ||
| // Violation: Custom error handling instead of using shared formatter | ||
| consola.error('Failed to decode transaction') | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| // Violation: Implements custom diamond cut formatting | ||
| function formatDiamondCut(decoded: unknown) { | ||
| // Violation: Should use formatDiamondCutSummary from safe-decode-utils.ts | ||
| consola.info('Diamond Cut Summary:') | ||
| // Custom formatting logic... | ||
| } | ||
|
|
||
| export function processSafeTransaction(data: Hex, chainId: number, network: string) { | ||
| // Violation: Should call formatDecodedTxDataForDisplay(data, { chainId, network }) | ||
| const decoded = decodeSafeTransaction(data) | ||
| formatDiamondCut(decoded) | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| /** | ||
| * Violation: Type definitions don't follow naming conventions. | ||
| * | ||
| * Convention violations: | ||
| * - Interfaces MUST start with `I` prefix (e.g., INetwork, ITransferResult) | ||
| * - Enums MUST use PascalCase with `Enum` suffix (e.g., EnvironmentEnum) | ||
| * - Type aliases use PascalCase without prefix (e.g., SupportedChain, HexString) | ||
| * | ||
| * This file violates by defining interfaces without `I` prefix and enums without `Enum` suffix. | ||
| */ | ||
|
|
||
| import { consola } from 'consola' | ||
|
|
||
| // Violation: Interface doesn't start with `I` prefix | ||
| export interface Network { | ||
| name: string | ||
| chainId: number | ||
| } | ||
|
|
||
| // Violation: Interface doesn't start with `I` prefix | ||
| export interface TransferResult { | ||
| success: boolean | ||
| txHash: string | ||
| } | ||
|
|
||
| // Violation: Enum doesn't have `Enum` suffix | ||
| export enum Environment { | ||
| staging = 'staging', | ||
| production = 'production', | ||
| } | ||
|
|
||
| // Violation: Enum doesn't have `Enum` suffix | ||
| export enum Status { | ||
| pending = 'pending', | ||
| completed = 'completed', | ||
| } | ||
|
|
||
| // Violation: Type alias uses lowercase (should be PascalCase) | ||
| export type networkName = string | ||
|
|
||
| // Violation: Type alias uses camelCase (should be PascalCase) | ||
| export type transferHash = string | ||
|
|
||
| export function badFunction(network: Network, result: TransferResult) { | ||
| // Function implementation... | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.