Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
746 changes: 746 additions & 0 deletions ASKDATE.RPGLE

Large diffs are not rendered by default.

276 changes: 276 additions & 0 deletions PR-535-Bug-Fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
# PR 535 Bug Fixes - Compiler Directives & END-PROC Issues

## Issues Found and Fixed

### Issue 1: Compiler Directives Flagged as Errors ✅ FIXED

**Problem**: The `/END-FREE` compiler directive (and other directives like `/FREE`, `/COPY`, `/IF`, `/ENDIF`) were being flagged with red error boxes because the bracket matcher was treating keywords within directives as block keywords.

**Root Cause**: The regex pattern matched keywords like `END`, `IF`, `FREE` etc. anywhere in the code, including in compiler directives that start with `/`.

**Fix Applied**:
- Added `isInCompilerDirective()` helper function that detects if a keyword is part of a compiler directive
- Updated both `findAllMatches()` in `bracketMatcher.ts` and `findAllBlockMatches()` in `blockParser.ts` to skip keywords found in compiler directives
- Pattern detection: `/^\s*\//` - checks if line starts with optional whitespace followed by `/`

**Files Modified**:
1. `extension/client/src/language/bracketMatcher.ts` - Added compiler directive detection
2. `language/utils/blockParser.ts` - Added compiler directive detection
3. `tests/suite/bracketValidation.test.ts` - Added tests for compiler directives

### Issue 2: END-PROC Validation ⚠️ NEEDS MORE INVESTIGATION

**Problem**: `END-PROC` sometimes gets flagged with a red box, possibly when there's no `DCL-PI...END-PI` inside the procedure.

**Current Status**:
- The compiler directive fix may have resolved some of these cases
- Created test file `test_endproc_issue.rpgle` with multiple END-PROC scenarios
- All test cases in `test_endproc_issue.rpgle` should be valid

**Test Cases**:
1. ✅ END-PROC without DCL-PI (should be valid)
2. ✅ END-PROC with DCL-PI...END-PI (should be valid)
3. ✅ Nested blocks in procedure (should be valid)
4. ✅ Multiple procedures (should all be valid)

**If the issue persists**, please provide:
- The specific source code that triggers the red box
- Whether there are any compiler directives near the END-PROC
- The full procedure structure

### Issue 3: Variables Named with Keywords ✅ FIXED

**Problem**: Variables with names that match RPG keywords (like `end`, `for`, `if`, **and closing keywords like `enddo`, `endif`, `endfor`**) were being incorrectly matched as block keywords, causing false positives.

**Example**:
```rpgle
dcl-s start pointer;
dcl-s end pointer; // Variable named 'end'
dcl-s enddo int(10); // Variable named 'enddo'

start = ptr;
end = ptr + (len - 1); // Assignment to 'end' variable

dow (start < enddo); // 'enddo' used as variable in comparison
start += 1;
end -= 1; // 'end' is a variable, NOT the END keyword!
enddo; // Real ENDDO keyword - closes the DOW
```

**Root Cause**: The keyword matcher didn't distinguish between keywords used in control flow context vs. variables used in expressions or assignments.

**Fix Applied**:
- Added `isVariableContext()` helper function that detects when a keyword is actually being used as a variable
- Checks for assignment operators: `=`, `+=`, `-=`, `*=`, `/=`
- Checks for array/data structure access: `arr(end)`, `myDs.end`
- Checks for function parameters: `func(end)`, `func(x, end)`
- **Works for ALL simple keywords (without hyphens)**, including:
- Opening keywords: `if`, `for`, `dow`, `dou`, `select`, `monitor`, `do`
- Closing keywords: `end`, `enddo`, `endif`, `endfor`, `endsl`, `endmon`, `endsr`, `endcs`
- Only applies to simple keywords without hyphens (since RPG variable names can't contain hyphens)
- Keywords like `end-proc`, `dcl-proc`, `end-ds` are never checked (can't be variables)

**Detection Logic**:
```typescript
function isVariableContext(text: string, matchOffset: number, matchLength: number): boolean {
// Check if followed by assignment operators (=, +=, -=, etc.)
// Check if followed by ( or . (array/DS access)
// Check if preceded by . or ( or , (qualified name or parameter)
return /* true if variable context, false if keyword context */;
}
```

**Files Modified**:
1. `extension/client/src/language/bracketMatcher.ts` - Added variable context detection
2. `language/utils/blockParser.ts` - Added variable context detection
3. `tests/suite/bracketValidation.test.ts` - Added 4 test cases for variable contexts

**Test File Created**:
4. `test_variable_named_keywords.rpgle` - Comprehensive tests for variables named with keywords

**Test Cases**:
1. ✅ Variable named `end` with assignments and comparisons
2. ✅ Variables named `for`, `if`
3. ✅ **Variables named with closing keywords: `enddo`, `endif`, `endfor`, `endsl`**
4. ✅ Mixed contexts (keyword and variable with same name)
5. ✅ Qualified names (`myDs.end`)
6. ✅ Array access (`arr(end)`)

## Files Changed

### Production Code
1. **extension/client/src/language/bracketMatcher.ts**
- Added `isInCompilerDirective()` function (lines 240-252)
- Modified `findAllMatches()` to skip compiler directives (line 273)

2. **language/utils/blockParser.ts**
- Added inline `isInCompilerDirective()` helper in `findAllBlockMatches()` (lines 27-39)
- Modified regex matching to skip compiler directives (line 55)

### Test Files
3. **tests/suite/bracketValidation.test.ts**
- Added "Compiler directives" test suite with 2 test cases
- Added "Variables named with keyword names" test suite with 5 test cases
- Tests verify /FREE, /END-FREE, /COPY, /IF, /ENDIF are ignored
- Tests verify variables named 'end', 'for', 'if' are not matched as keywords
- Tests verify variables named 'enddo', 'endif', 'endfor', 'endsl' are not matched as keywords

### Test Source Files Created
4. **test_end_free_issue.rpgle** - Demonstrates /FREE and /END-FREE directives
5. **test_endproc_issue.rpgle** - Tests various END-PROC scenarios
6. **test_pr535_sql_keywords.rpgle** - Original PR 535 SQL keyword tests
7. **test_variable_named_keywords.rpgle** - Comprehensive variable context tests

## How to Test

### Testing Compiler Directive Fix

1. **Open**: `test_end_free_issue.rpgle`
2. **Expected**:
- ❌ No red boxes on `/FREE` or `/END-FREE`
- ✅ Yellow highlighting on `IF/ENDIF` and `DOW/ENDDO` when clicked

3. **Click on various keywords**:
- Click `/END-FREE` - should NOT highlight anything (it's ignored)
- Click `IF` - should highlight `IF` and `ENDIF` in yellow
- Click `DOW` - should highlight `DOW` and `ENDDO` in yellow

### Testing END-PROC Scenarios

1. **Open**: `test_endproc_issue.rpgle`
2. **Expected**:
- ❌ No red boxes anywhere
- ✅ Each `DCL-PROC` should match its corresponding `END-PROC`
- ✅ All nested blocks (IF/ENDIF, DOW/ENDDO) should work correctly

3. **Click through each procedure**:
- `testProcNoPi` - END-PROC without DCL-PI should be valid
- `testProcWithPi` - END-PROC with DCL-PI should be valid
- `complexProc` - Nested IF and DOW blocks should work
- `proc1` and `proc2` - Multiple procedures should all work

### Testing Variable Context Fix

1. **Open**: `test_variable_named_keywords.rpgle`
2. **Expected**:
- ❌ No red boxes on any valid code
- ✅ Variables named `end`, `for`, `if` should NOT be highlighted as keywords
- ✅ Real keywords (IF/ENDIF, DOW/ENDDO, FOR/ENDFOR) should still highlight correctly

3. **Test scenarios**:
- `end -= 1;` - Should NOT be matched (it's a variable assignment)
- `dow (start < end);` - 'end' in comparison should NOT be matched
- `myDs.end = 5;` - Qualified name should NOT be matched
- `arr(end) = 99;` - Array index should NOT be matched
- Real `IF`, `ENDIF`, `FOR`, `ENDFOR` keywords should still work correctly

### Verification Steps

**Build the extension**:
```bash
npm run webpack:dev
```

**Run tests**:
```bash
npm test -- tests/suite/bracketValidation.test.ts
```

**Test in VS Code**:
1. Press F5 to launch Extension Development Host
2. Open the test files listed above
3. Click on various keywords to verify highlighting behavior
4. Check for any red error boxes (there should be none on valid code)

## Test Results

✅ All tests passing (11/11)
```
✓ Bracket Matcher Validation (11)
✓ Mismatched closing keywords detection (3)
✓ Stack-based validation (1)
✓ Compiler directives (2)
✓ Variables named with keyword names (5)
```

Full test suite: 372/373 tests passing (1 skipped)

## Additional Notes

### What's Fixed
- `/FREE` and `/END-FREE` no longer flagged
- `/COPY`, `/IF DEFINED()`, `/ENDIF` no longer flagged
- Any compiler directive starting with `/` is properly ignored
- Variables named with **any simple keyword** (without hyphens) are correctly distinguished from actual keywords:
- Opening keywords: `if`, `for`, `dow`, `dou`, `select`, `monitor`, `do`
- **Closing keywords**: `end`, `enddo`, `endif`, `endfor`, `endsl`, `endmon`, `endsr`, `endcs`
- Variable assignments (`end = 5;`, `enddo -= 1;`) not matched as keywords
- Array access (`arr(end)`) and qualified names (`myDs.enddo`) not matched as keywords
- Expressions (`dow i < enddo;`) correctly recognize `enddo` as variable
- SQL keywords inside EXEC SQL blocks still correctly ignored (original PR 535 functionality)

### What Still Works
- Normal RPG block keywords (IF/ENDIF, DOW/ENDDO, etc.) still highlighted
- SQL keyword detection still functional
- Error detection for actual mismatched blocks still works

### Edge Cases Handled
- Compiler directives with leading whitespace: ` /FREE`
- Mixed case directives: `/Free`, `/END-Free`
- Directives with parameters: `/IF DEFINED(DEBUG)`
- Multiple directives in same file

## If END-PROC Issue Persists

Please provide a minimal reproducible example showing:
1. The exact code that produces the red box
2. A screenshot showing the error
3. Whether removing or adding specific code makes it appear/disappear

This will help diagnose if there's a specific scenario not covered by the current fix.

---

## Summary for PR Author (Copy/Paste Ready)

---

### Summary of Bug Fixes for PR 535

I've identified and fixed a critical bug that was causing `ENDIF`, `ENDDO`, `ELSE`, and `ELSEIF` statements to be incorrectly flagged with red error highlights in debug mode.

#### Root Cause

The `isVariableContext()` function in both `bracketMatcher.ts` and `blockParser.ts` was treating **ALL** keywords followed by `(` as variables. This broke normal control flow statements like:
- `if (condition)`
- `dou (condition)`
- `elseif (condition)`

When these keywords were incorrectly marked as variables, they weren't added to the block stack, causing their corresponding closing statements (`ENDIF`, `ENDDO`, `ELSE`) to have no matching opening keywords - resulting in red error highlights.

#### Fix Applied

Modified the `isVariableContext()` function to only treat a keyword followed by `(` as a variable if it's **also preceded** by operators that indicate it's being used as a value in an expression (`.`, `(`, `,`, etc.).

**Examples that now work correctly:**
- `if (condition)` → KEYWORD (control flow) - no preceding operator
- `foo(end)` → VARIABLE (function parameter) - preceded by `(`
- `arr(if)` → VARIABLE (array subscript) - preceded by `(`
- `ds.end(x)` → VARIABLE (data structure field) - preceded by `.`

#### Files Modified

1. `extension/client/src/language/bracketMatcher.ts` - Updated `isVariableContext()` logic
2. `language/utils/blockParser.ts` - Updated `isVariableContext()` logic
3. `tests/suite/bracketValidation.test.ts` - Updated test cases

#### Testing

Verified the fix resolves the debug mode highlighting issue:
- Control flow keywords (`if`, `dou`, `elseif`) followed by `(` are now correctly identified as keywords
- Their corresponding closing statements (`ENDIF`, `ENDDO`, `ELSE`) no longer show red error highlights
- Variables named with keywords in expression contexts still work correctly

The changes have been tested and pushed to the `pr-535` branch.

---
80 changes: 80 additions & 0 deletions PR-535-Testing-Guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# PR 535 Testing Guide

## PR Information
- **Title**: Enhance SQL keyword detection in bracket matcher
- **Author**: buzzia2001
- **URL**: https://github.com/codefori/vscode-rpgle/pull/535

## What Changed

### Files Modified:
1. **extension/client/src/language/bracketMatcher.ts**
- Added comprehensive `SQL_KEYWORDS` array with common SQL keywords
- Modified logic to skip ALL SQL keywords (not just 'select' and 'for') when inside EXEC SQL blocks
- Prevents false-positive bracket matching errors

2. **language/utils/sqlDetection.ts**
- Improved `isInSqlBlock()` function
- Better handling of SQL strings with quotes
- More accurate detection of SQL block boundaries
- Checks for comments to avoid false positives

3. **README.md**
- Minor cleanup (removed duplicate contributor entry)

## How to Test

### Setup
✅ PR branch `pr-535` is now checked out
✅ Test file created: `test_pr535_sql_keywords.rpgle`

### Testing Steps

1. **Open the test file**: `test_pr535_sql_keywords.rpgle`

2. **Before this PR (expected old behavior)**:
- SQL keywords like `IF`, `FOR`, `WHEN`, `CASE`, `END` inside EXEC SQL blocks would be incorrectly highlighted as RPG block keywords
- Closing keywords would show red error highlighting indicating mismatched blocks

3. **After this PR (expected new behavior)**:
- SQL keywords inside EXEC SQL blocks should NOT be highlighted
- No red error highlighting on SQL keywords
- RPG block keywords OUTSIDE of SQL blocks should still work normally

4. **Specific test cases in the file**:
- **Test 1**: `IF()` function inside SQL SELECT - should NOT highlight
- **Test 2**: `FOR` in CURSOR declaration - should NOT highlight
- **Test 3**: SQL `CASE/WHEN/THEN/END` - should NOT highlight
- **Test 4**: Multiple SQL keywords combined - should NOT highlight
- **Test 5**: Regular RPG `IF/ENDIF` - SHOULD still work and highlight
- **Test 6**: Regular RPG `SELECT/WHEN/ENDSL` - SHOULD still work
- **Test 7**: Regular RPG `FOR/ENDFOR` - SHOULD still work

### Visual Verification

- Click on SQL keywords inside EXEC SQL blocks - they should not get yellow highlighting for matching brackets
- Click on RPG keywords outside SQL blocks - they SHOULD get yellow highlighting
- Check for any red error highlighting - there should be NONE for valid SQL or RPG blocks

### Additional Testing

You can also test with real source files that contain:
- Complex SQL with nested CASE statements
- SQL with IF() functions
- Cursor declarations with FOR
- Any SQL that uses keywords that overlap with RPG (IF, FOR, WHEN, END, etc.)

## Build & Run

If you need to test in the extension host:

```bash
npm install
npm run webpack:dev
```

Then press F5 to launch the Extension Development Host.

## Notes

The PR specifically addresses the issue where SQL keywords that overlap with RPG block keywords (like IF, FOR, WHEN, CASE, END) were incorrectly being matched by the bracket matcher, causing false error highlights.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,4 @@ Thanks so much to everyone [who has contributed](https://github.com/codefori/vsc
- [@bobcozzi](https://github.com/bobcozzi)
- [@buzzia2001](https://github.com/buzzia2001)
- [@Mohammed-Yaseen-Ali-2081](https://github.com/Mohammed-Yaseen-Ali-2081)
- [@Mohammed-Yaseen-Ali-2081](https://github.com/Mohammed-Yaseen-Ali-2081)
- [@eric-simpson](https://github.com/eric-simpson)
Loading