Skip to content
Draft
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
28 changes: 25 additions & 3 deletions src/compiler/code-generator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { OPCODE } from '../ClassFile/constants/instructions'
import { ACCESS_FLAGS } from '../ClassFile/types'
import { ExceptionHandler, AttributeInfo } from '../ClassFile/types/attributes'
import { FIELD_FLAGS } from '../ClassFile/types/fields'
import { METHOD_FLAGS } from '../ClassFile/types/methods'
Expand Down Expand Up @@ -1375,14 +1376,35 @@ const codeGenerators: { [type: string]: (node: Node, cg: CodeGenerator) => Compi
const { stackSize: exprStackSize, resultType } = compile(expression, cg)
let maxStack = exprStackSize

// If the expression is an enum type, invoke ordinal() to convert to int and then continue
let _resultType = resultType
if (_resultType && _resultType.startsWith('L') && _resultType !== 'Ljava/lang/String;') {
const clean = _resultType.replace(/^L|;$/g, '')
try {
const classInfo = cg.symbolTable.queryClass(clean)
if (classInfo.accessFlags & ACCESS_FLAGS.ACC_ENUM) {
// call java.lang.Enum.ordinal() (returns int)
cg.code.push(
OPCODE.INVOKEVIRTUAL,
0,
cg.constantPoolManager.indexMethodrefInfo('java/lang/Enum', 'ordinal', '()I')
)
_resultType = 'I'
maxStack = Math.max(maxStack, exprStackSize + 1)
}
} catch (e) {
// ignore: not a known class
}
}

const caseLabels: Label[] = cases.map(() => cg.generateNewLabel())
const defaultLabel = cg.generateNewLabel()
const endLabel = cg.generateNewLabel()

// Track the switch statement's end label
cg.switchLabels.push(endLabel)

if (['I', 'B', 'S', 'C'].includes(resultType)) {
if (['I', 'B', 'S', 'C'].includes(_resultType)) {
const caseValues: number[] = []
const caseLabelMap: Map<number, Label> = new Map()
let hasDefault = false
Expand Down Expand Up @@ -1556,7 +1578,7 @@ const codeGenerators: { [type: string]: (node: Node, cg: CodeGenerator) => Compi
}

endLabel.offset = cg.code.length
} else if (resultType === 'Ljava/lang/String;') {
} else if (_resultType === 'Ljava/lang/String;') {
// **String Switch Handling**
const hashCaseMap: Map<number, Label> = new Map()

Expand Down Expand Up @@ -1708,7 +1730,7 @@ const codeGenerators: { [type: string]: (node: Node, cg: CodeGenerator) => Compi
endLabel.offset = cg.code.length
} else {
throw new Error(
`Switch statements only support byte, short, int, char, or String types. Found: ${resultType}`
`Switch statements only support byte, short, int, char, String, or enum types. Found: ${_resultType}`
)
}

Expand Down
3 changes: 2 additions & 1 deletion src/compiler/compiler-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { ClassModifier, FieldModifier, MethodModifier } from '../ast/types/class
const classAccessFlagMap = new Map([
['public', ACCESS_FLAGS.ACC_PUBLIC],
['final', ACCESS_FLAGS.ACC_FINAL],
['abstract', ACCESS_FLAGS.ACC_ABSTRACT]
['abstract', ACCESS_FLAGS.ACC_ABSTRACT],
['enum', ACCESS_FLAGS.ACC_ENUM]
Comment on lines +9 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Confirm that 'enum' reaches classModifier and that ACC_ENUM can be set.
set -euo pipefail

echo '=== ClassModifier type definition ==='
rg -nP -C 4 '\bClassModifier\b' --type=ts src/ast/types

echo '=== Where classModifier is populated ==='
rg -nP -C 5 'classModifier' --type=ts src

echo '=== Any enum handling in the AST extractor ==='
rg -nP -C 5 -i 'enumdeclaration|enumconstant' --type=ts src/ast

echo '=== ACC_ENUM definition and all uses ==='
rg -nP -C 3 'ACC_ENUM' --type=ts src

Repository: source-academy/java-slang

Length of output: 50381


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== AST class types ==='
cat -n src/ast/types/classes.ts

echo '=== Enum-related source symbols ==='
rg -n -i --glob '*.ts' '\benum\b|EnumDeclaration|enumConstant' src --glob '!**/__tests__/**' | head -200

echo '=== Class declaration extraction and parser definitions ==='
rg -n -i --glob '*.ts' 'NormalClassDeclaration|classModifier|classDeclaration' src/parser src/ast src 2>/dev/null | head -240

Repository: source-academy/java-slang

Length of output: 30302


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Compiler inputs and class extraction ==='
rg -n -C 6 --glob '*.ts' 'classModifier|ClassDeclaration|NormalClassDeclaration|generateClassAccessFlags' src/compiler src/ast

echo '=== Legacy compiler grammar around class declarations ==='
sed -n '130,275p' src/compiler/grammar.ts

echo '=== Legacy class extractor ==='
cat -n src/ast/astExtractor/class-extractor.ts

echo '=== Compiler entry point ==='
sed -n '1,140p' src/compiler/compiler.ts

echo '=== New AST enum declaration and class declaration types ==='
sed -n '230,270p' src/types/ast/specificationTypes.ts
sed -n '460,485p' src/types/ast/specificationTypes.ts
sed -n '360,390p' src/types/ast/extractor.ts
sed -n '690,715p' src/types/ast/extractor.ts

Repository: source-academy/java-slang

Length of output: 50382


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Legacy grammar: class modifiers and enum token ==='
sed -n '160,190p' src/compiler/grammar.ts
sed -n '490,525p' src/compiler/grammar.ts

echo '=== Legacy parser entry points ==='
fd -t f -e ts . src/ast src/compiler | sort | grep -E 'parser|grammar|extractor'
rg -n -C 8 'from .*compiler|grammar|parse\(' src/compiler src/ast --glob '*.ts' | head -180

echo '=== Class modifier implementation ==='
rg -n -C 10 'ClassModifier|possibleModifiers|classModifier\(ctx' src/compiler/grammar.ts src/ast/astExtractor/class-extractor.ts src/ast/parser* 2>/dev/null

Repository: source-academy/java-slang

Length of output: 21495


Model enum declarations separately from class modifiers

ClassModifier and ClassModifier* do not include enum. The compiler therefore cannot place enum in classModifier; its grammar does not parse enum declarations as NormalClassDeclaration. This entry cannot set ACC_ENUM. Add enum declaration support and set the flag from the declaration kind, or remove this unreachable mapping.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/compiler/compiler-utils.ts` around lines 9 - 10, Remove the unreachable
enum entry from the class-modifier mapping, or implement enum declaration
parsing separately from NormalClassDeclaration and assign ACCESS_FLAGS.ACC_ENUM
from the declaration kind rather than classModifier. Ensure ClassModifier
remains limited to supported class modifiers.

])

export function generateClassAccessFlags(modifiers: Array<ClassModifier>) {
Expand Down
48 changes: 47 additions & 1 deletion src/types/checker/__tests__/switchStatements.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { check } from '..'
import { parse } from '../../ast'
import { IncompatibleTypesError, TypeCheckerError } from '../../errors'
import { IncompatibleTypesError, SelectorTypeNotAllowedError, TypeCheckerError } from '../../errors'
import { Type } from '../../types/type'

const createProgram = (statement: string) => `
Expand All @@ -27,6 +27,27 @@ const testcases: {
`,
result: { type: null, errors: [] }
},
{
input: `
String selector = "Tuesday";
switch(selector) {
case "Tuesday": {
selector = "Wednesday";
}
default:
}
`,
result: { type: null, errors: [] }
},
{
input: `
Boolean selector = true;
switch(selector) {
default: {}
}
`,
result: { type: null, errors: [new SelectorTypeNotAllowedError()] }
},
{
input: `
int selector = 1;
Expand All @@ -52,6 +73,31 @@ const testcases: {
}
`,
result: { type: null, errors: [new IncompatibleTypesError()] }
},
{
input: `
enum Color { RED, BLUE }
Color selector = Color.RED;
switch(selector) {
case Color.RED: {
selector = Color.BLUE;
}
default: {}
}
`,
result: { type: null, errors: [] }
},
{
input: `
enum Color { RED, BLUE }
enum Other { X }
Color selector = Color.RED;
switch(selector) {
case Other.X: {}
default: {}
}
`,
result: { type: null, errors: [new IncompatibleTypesError()] }
Comment on lines +76 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

These tests use qualified enum constants in case labels, which Java does not permit.

Java requires the unqualified constant name in an enum switch label. case Color.RED: is a compile error in javac; the legal form is case RED:. Both new tests use the qualified form.

Two consequences:

  1. The positive test at lines 77-89 asserts that an invalid Java program type checks successfully.
  2. The negative test at lines 90-100 passes for the wrong reason. Real javac rejects case Other.X: because it is qualified, not because of a type mismatch.

The unqualified form is the one that matters, and it is untested. Resolving a bare RED requires the checker to bring the selector enum's constants into the case-label scope. Nothing in src/types/checker/index.ts lines 727-740 does that, so case RED: would likely fail with CannotFindSymbolError.

Add a test using case RED: to confirm the behavior.

Both tests also declare the enum inside the main method body, because createProgram inserts the statement there. No test declares a top-level enum, so the top-level registration path in src/types/checker/prechecks.ts is uncovered.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/types/checker/__tests__/switchStatements.test.ts` around lines 76 - 100,
Update the switch statement tests to use Java-valid unqualified enum labels:
change the positive case to case RED and retain the incompatible case with an
unqualified constant from Other, then add coverage for resolving selector enum
constants in case-label scope. Also add a test declaring an enum at top level to
exercise the registration path in prechecks alongside the existing method-local
declarations.

}
]

Expand Down
4 changes: 3 additions & 1 deletion src/types/checker/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ const GLOBAL_TYPE_ENVIRONMENT: { [key: string]: Type } = {
// Hard coded variables
System: SYSTEM_CLASS,
Throwable: new NonPrimitives.Throwable(),
Exception: new NonPrimitives.Exception()
Exception: new NonPrimitives.Exception(),
// enum base type
Enum: new ClassType('Enum')
}

export class Frame {
Expand Down
108 changes: 96 additions & 12 deletions src/types/checker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const isCastCompatible = (fromType: Type, toType: Type): boolean => {
const fromName = fromType.constructor.name;
const toName = toType.constructor.name;

console.log(fromName, toName);

return !(fromName === 'char' && toName !== 'int');
}
Expand Down Expand Up @@ -384,7 +383,6 @@ export const typeCheckBody = (node: Node, frame: Frame = Frame.globalFrame()): R
return newResult(null, errors)
}
case 'InstanceofExpression': {
console.log(node)
return OK_RESULT
}
case 'BinaryLiteral':
Expand Down Expand Up @@ -584,6 +582,88 @@ export const typeCheckBody = (node: Node, frame: Frame = Frame.globalFrame()): R
}
return newResult(null, errors)
}
case 'EnumDeclaration': {
const errors: TypeCheckerError[] = []
const classType = frame.getType(node.typeIdentifier.identifier, node.typeIdentifier.location)
if (classType instanceof TypeCheckerError) return newResult(null, [classType])
if (!(classType instanceof ClassType)) throw new Error('enum type retrieved should be ClassImpl')

const classFrame = frame.newChildFrame()
classFrame.setClass(classType)
classType.mapFields((name, type) => {
const error = classFrame.setVariable(name, type, { startLine: -1, startOffset: -1 })
if (error) errors.push(error)
})
if (errors.length > 0) return newResult(null, errors)

const bodyDecls = node.enumBody.enumBodyDeclarations?.classBodyDeclaration || []
let numFieldDeclarations = 0
let numMethodDeclarations = 0
for (let i = 0; i < bodyDecls.length; i++) {
const bodyDeclaration = bodyDecls[i]
switch (bodyDeclaration.kind) {
case 'ConstructorDeclaration': {
const methodFrame = classFrame.newChildFrame()
const constructor = classType.getConstructor(i - numFieldDeclarations - numMethodDeclarations)
const constructorMethodErrors: TypeCheckerError[] = []
constructor.mapParameters((name, type, isVarargs) => {
const error = methodFrame.setVariable(name, type, { startLine: -1, startOffset: -1 })
if (error) constructorMethodErrors.push(error)
})
if (constructorMethodErrors.length > 0) {
errors.push(...constructorMethodErrors)
break
}
const { errors: checkErrors } = typeCheckBody(bodyDeclaration.constructorBody, methodFrame)
if (checkErrors.length > 0) errors.push(...checkErrors)
break
}
case 'FieldDeclaration': {
for (const variableDeclarator of (bodyDeclaration as any).variableDeclaratorList.variableDeclarators) {
const field = classType.accessField(variableDeclarator.variableDeclaratorId.identifier.identifier, variableDeclarator.variableDeclaratorId.identifier.location)
if (field instanceof TypeCheckerError) throw new Error('field should exist in enum')
const initializer = variableDeclarator.variableInitializer
if (initializer) {
const type = createArrayType(field, initializer, expression => {
const result = typeCheckBody(expression, frame)
if (result.errors.length > 0) return result.errors[0]
if (!result.currentType) throw new Error('array initializer expression should have a type')
return result.currentType
})
if (type instanceof TypeCheckerError) errors.push(type)
}
}
break
}
case 'MethodDeclaration': {
const methodIdentifier = (bodyDeclaration as any).methodHeader.methodDeclarator.identifier
const methodName = methodIdentifier.identifier
const overloadIndex = bodyDecls
.filter((n: any) => n.kind === 'MethodDeclaration' && (n as any).methodHeader.methodDeclarator.identifier.identifier === methodName)
.findIndex(n => n === bodyDeclaration)
const method = classType.getMethod(methodName)[overloadIndex]
const methodFrame = classFrame.newChildFrame()
const methodErrors: TypeCheckerError[] = []
methodFrame.setReturnType(method.getReturnType())
method.mapParameters((name, type, isVarargs) => {
const error = methodFrame.setVariable(name, type, { startLine: -1, startOffset: -1 })
if (error) methodErrors.push(error)
})
if (methodErrors.length > 0) {
errors.push(...methodErrors)
break
}
const { errors: checkErrors } = typeCheckBody((bodyDeclaration as any).methodBody, methodFrame)
if (checkErrors.length > 0) errors.push(...checkErrors)
break
}
}

if (bodyDeclaration.kind === 'FieldDeclaration') numFieldDeclarations += 1
if (bodyDeclaration.kind === 'MethodDeclaration') numMethodDeclarations += 1
}
return newResult(null, errors)
}
case 'OrdinaryCompilationUnit': {
const typeCheckErrors = node.topLevelClassOrInterfaceDeclarations
.map(declaration => typeCheckBody(declaration, frame))
Expand Down Expand Up @@ -644,16 +724,20 @@ export const typeCheckBody = (node: Node, frame: Frame = Frame.globalFrame()): R
const switchBlockFrame = frame.newChildFrame()
for (const group of node.switchBlock.switchBlockStatementGroups) {
for (const switchLabel of group.switchLabels) {
if ('caseConstant' in switchLabel) {
const checkResult = typeCheckBody(
switchLabel.caseConstant as CaseConstant,
switchBlockFrame
)
if (checkResult.hasErrors) return checkResult
if (!checkResult.currentType)
throw new TypeCheckerInternalError('Switch case constant should have a type.')
if (expressionCheck.currentType.canBeAssigned(checkResult.currentType)) continue
return newResult(null, [new IncompatibleTypesError(switchLabel.location)])
// Support both singular 'caseConstant' and plural 'caseConstants' AST shapes
const caseConstants: CaseConstant[] = []
if ('caseConstant' in switchLabel && (switchLabel as any).caseConstant) caseConstants.push((switchLabel as any).caseConstant as CaseConstant)
if ('caseConstants' in switchLabel && (switchLabel as any).caseConstants) caseConstants.push(...((switchLabel as any).caseConstants as CaseConstant[]))
if (caseConstants.length > 0) {
for (const caseConst of caseConstants) {
const checkResult = typeCheckBody(caseConst, switchBlockFrame)
if (checkResult.hasErrors) return checkResult
if (!checkResult.currentType)
throw new TypeCheckerInternalError('Switch case constant should have a type.')
const assignable = expressionCheck.currentType.canBeAssigned(checkResult.currentType)
if (assignable) continue
return newResult(null, [new IncompatibleTypesError(switchLabel.location)])
}
}
}
if (group.blockStatements) {
Expand Down
Loading
Loading