samreid
commented
5 months ago @matthew-blackman and I wrote a draft script to help us with access modifiers:
```js
// Iterate through all the public and protected access modifiers within the js/ directory for the specified repo.
// For each one:
// 1. Change it to private. Then run the type checker to see if any errors are thrown.
// - If it passes the type checker, keep it.
// - If it fails, escalate from private => protected => public, testing at each level.
import { Project } from 'ts-morph';
import { execSync } from 'child_process';
// Initialize a new ts-morph Project
const project = new Project( {
// Assuming tsconfig.json is in the root, adjust if necessary
tsConfigFilePath: 'tsconfig.json'
} );
// Function to tighten accessibility annotations
async function tightenAccessModifiers() {
const sourceFiles = project.getSourceFiles( 'js/**/*.ts' ); // Adjust the glob pattern as necessary
for ( const sourceFile of sourceFiles ) {
const classes = sourceFile.getClasses();
for ( const classDeclaration of classes ) {
// if ( classDeclaration.getName() !== 'PDLUtils' ) {
// continue;
// }
console.log( `# Processing class: ${classDeclaration.getName()}` );
const members = [
...classDeclaration.getInstanceProperties(),
...classDeclaration.getInstanceMethods(),
...classDeclaration.getStaticProperties(),
...classDeclaration.getStaticMethods()
];
for ( const member of members ) {
// if ( member.getName() !== 'meanSpeedProperty' ) {
// continue;
// }
console.log( member.getScope() + ' ' + member.getName() );
if ( member.getScope() === 'public' || member.getScope() === 'protected' ) { // Correct way to check for public modifier using ts-morph
// console.log( ` Found public member: ${member.getName()}, attempting to set to private.` );
// Try setting to private
member.setScope( 'private' );
await sourceFile.save();
if ( !isBuildSuccessful() ) {
// console.log( ` Setting ${member.getName()} to private failed, attempting to set to protected.` );
// If not successful, try protected
member.setScope( 'protected' );
await sourceFile.save();
if ( !isBuildSuccessful() ) {
// console.log( ` Setting ${member.getName()} to protected failed, reverting to public.` );
// If still not successful, revert to public
member.setScope( 'public' );
await sourceFile.save();
}
else {
console.log( ` Successfully changed ${member.getName()} to protected.` );
}
}
else {
console.log( ` Successfully changed ${member.getName()} to private.` );
}
}
}
}
// Optionally save the changes here if needed
await sourceFile.save();
}
}
function isBuildSuccessful() {
// console.log( 'isBuildSuccessful???' );
try {
// Specify the path to the TypeScript compiler you want to use
const tscPath = '/Users/samreid/phet/root/chipper/node_modules/typescript/bin/tsc';
// Run the specified TypeScript compiler in the current directory
execSync( `node ${tscPath}` );
// If tsc exits without error, the build is successful
// console.log( 'Build successful' );
return true;
}
catch( error ) {
// If tsc exits with an error (non-zero exit code), the build failed
// console.error( 'TypeScript compilation failed' );
return false;
}
}
// Run the script
tightenAccessModifiers().then( () => console.log( 'Finished processing files.' ) );
```
For code review #32 ...
There are quite a few things with accessibility
publicthat don't need to be part of the public API, and should beprotectedorprivate.For example in Field.ts:
Then there are constructors for base classes that are not intended to be directly instantiated, and should therefore be
protected: VSMModel, SMModel, HeatMapToolNode, and probably others.Was your default to start with
publicand then narrow later? If so, you might want to reconsider that approach -- start withprivateand widen as needed, so that your IDE/TypeScript helps get the annotation correct.Also recommended to review this sim again to tighten up the accessibility annotations.